-
Notifications
You must be signed in to change notification settings - Fork 15
Add PR health tool #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add PR health tool #142
Changes from 5 commits
d5be3c0
72ab236
0b2e3af
637099c
1159eef
c84de39
41c6497
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| namespace NuGetDashboard; | ||
|
|
||
| public class DashboardService(GitHubClient client, int windowDays = 14) | ||
| { | ||
| private static readonly HashSet<string> TeamMembers = new(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "nkolev92", "zivkan", "jeffkl", "donnie-msft", "kartheekp-ms", | ||
| "martinrrm", "jebriede", "Nigusu-Allehu", "aortiz-msft" | ||
| }; | ||
|
|
||
| public async Task<DashboardData> BuildDashboardAsync() | ||
| { | ||
| var now = DateTime.UtcNow; | ||
| var windowAgo = now.AddDays(-windowDays); | ||
|
|
||
| Console.Write($" Fetching PRs merged in the past {windowDays} days... "); | ||
| var rawPRs = (await client.SearchMergedPRsAsync(windowAgo, now)) | ||
| .Where(p => TeamMembers.Contains(p.Author)) | ||
| .ToList(); | ||
| Console.WriteLine($"{rawPRs.Count} found."); | ||
|
|
||
| var callsNeeded = rawPRs.Count * 2; // timeline + reviews per PR | ||
| var (coreRemaining, coreLimit) = await client.GetCoreRateLimitAsync(); | ||
| Console.WriteLine($" Core API budget: {coreRemaining}/{coreLimit} remaining, {callsNeeded} needed."); | ||
| if (coreRemaining < callsNeeded) | ||
| throw new InvalidOperationException( | ||
| $"GitHub core API rate limit too low: {coreRemaining} remaining, {callsNeeded} needed.\n" + | ||
| $" → Create a free classic token at github.com/settings/tokens/new?type=classic (no scopes needed)"); | ||
|
|
||
| Console.Write($" Enriching {rawPRs.Count} PRs... "); | ||
| var prs = await EnrichAsync(rawPRs); | ||
| Console.WriteLine("done."); | ||
|
|
||
| return new DashboardData( | ||
| DateRange: $"{windowAgo:MMM d} \u2013 {now:MMM d, yyyy}", | ||
| AsOf: now.ToString("MMMM d, yyyy") + " UTC", | ||
| WindowDays: windowDays, | ||
| Metrics: ComputeMetrics(prs), | ||
| SlowPRs: prs.Where(p => p.HoursToMerge > 72).OrderByDescending(p => p.HoursToMerge).ToList(), | ||
| AllPRs: prs.OrderBy(p => p.MergedAt).ToList()); | ||
| } | ||
|
|
||
| private async Task<List<PRRecord>> EnrichAsync(List<RawPR> prs) | ||
| { | ||
| var results = new List<PRRecord>(); | ||
| foreach (var raw in prs) | ||
| { | ||
| var readyAt = await client.GetReadyTimeAsync(raw.Number); | ||
| var effectiveStart = readyAt ?? raw.CreatedAt; | ||
| var approvedAt = await client.GetFirstApprovalAtAsync(raw.Number); | ||
|
|
||
| results.Add(new PRRecord( | ||
| raw.Number, raw.Title, raw.Url, raw.Author, | ||
| raw.CreatedAt, effectiveStart, raw.MergedAt, | ||
| HoursToMerge: Math.Max(0, (raw.MergedAt - effectiveStart).TotalHours), | ||
| FirstApprovalHours: approvedAt.HasValue ? (approvedAt.Value - effectiveStart).TotalHours : null, | ||
| FirstApprovedAt: approvedAt)); | ||
|
|
||
| await Task.Delay(200); // avoid GitHub secondary rate limits | ||
| } | ||
| return results; | ||
| } | ||
|
|
||
| private static DashboardMetrics ComputeMetrics(List<PRRecord> prs) | ||
| { | ||
| if (prs.Count == 0) return new DashboardMetrics(0, 0, 0, 0); | ||
|
|
||
| var sorted = prs.Select(p => p.HoursToMerge).OrderBy(x => x).ToList(); | ||
| var n = sorted.Count; | ||
| var median = n % 2 == 0 ? (sorted[n / 2 - 1] + sorted[n / 2]) / 2.0 : sorted[n / 2]; | ||
| var reviewed = prs.Where(p => p.FirstApprovalHours.HasValue).ToList(); | ||
|
|
||
| return new DashboardMetrics( | ||
| TotalPRs: prs.Count, | ||
| MedianHoursToComplete: Math.Round(median, 1), | ||
| PercentApprovedUnder24h: reviewed.Count > 0 | ||
| ? Math.Round((double)reviewed.Count(p => p.FirstApprovalHours! < 24) / reviewed.Count * 100, 1) : 0, | ||
| PercentMergedUnder24h: Math.Round((double)prs.Count(p => p.HoursToMerge < 24) / prs.Count * 100, 1)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,198 @@ | ||||||||||||||||||||||||||
| using System.Net.Http.Headers; | ||||||||||||||||||||||||||
| using System.Text.Json; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| namespace NuGetDashboard; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public sealed class GitHubClient : IDisposable | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| private readonly HttpClient _http; | ||||||||||||||||||||||||||
| private const string Repo = "NuGet/NuGet.Client"; | ||||||||||||||||||||||||||
| private int _rateLimitRemaining = int.MaxValue; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public int RateLimitRemaining => _rateLimitRemaining; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Calls /rate_limit (free — not counted against quota) and returns | ||||||||||||||||||||||||||
| /// the core API remaining budget, which is what timeline/review calls consume. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| public async Task<(int remaining, int limit)> GetCoreRateLimitAsync() | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| using var resp = await _http.GetAsync("rate_limit"); | ||||||||||||||||||||||||||
| if (!resp.IsSuccessStatusCode) return (int.MaxValue, int.MaxValue); | ||||||||||||||||||||||||||
|
Comment on lines
+17
to
+21
|
||||||||||||||||||||||||||
| /// </summary> | |
| public async Task<(int remaining, int limit)> GetCoreRateLimitAsync() | |
| { | |
| using var resp = await _http.GetAsync("rate_limit"); | |
| if (!resp.IsSuccessStatusCode) return (int.MaxValue, int.MaxValue); | |
| /// Throws if rate-limit validation could not be performed. | |
| /// </summary> | |
| public async Task<(int remaining, int limit)> GetCoreRateLimitAsync() | |
| { | |
| using var resp = await _http.GetAsync("rate_limit"); | |
| TrackRateLimit(resp); | |
| await ThrowIfErrorAsync(resp, "core rate limit"); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| namespace NuGetDashboard; | ||
|
|
||
| public static class HtmlGenerator | ||
| { | ||
| public static void Generate(DashboardData data, string outputPath) | ||
| { | ||
| var sb = new System.Text.StringBuilder(); | ||
|
|
||
| sb.AppendLine(""" | ||
| <!DOCTYPE html> | ||
| <html><head><meta charset="utf-8"> | ||
| <style> | ||
| body { font-family: Calibri, Arial, sans-serif; font-size: 11pt; color: #1F1F1F; max-width: 900px; } | ||
| h1 { font-size: 14pt; color: #1F3864; } | ||
| h2 { font-size: 12pt; color: #2E74B5; margin-top: 24px; } | ||
| table { border-collapse: collapse; width: 100%; margin-bottom: 12px; } | ||
| th { background: #1F3864; color: #fff; padding: 6px 10px; text-align: left; } | ||
| td { padding: 5px 10px; border-bottom: 1px solid #D9D9D9; } | ||
| tr:nth-child(even) td { background: #F2F2F2; } | ||
| a { color: #0563C1; } | ||
| .key p { margin: 2px 0; } | ||
| </style> | ||
| </head><body> | ||
| """); | ||
|
|
||
| sb.AppendLine($"<h1>PR health over the past {data.WindowDays} days</h1>"); | ||
|
|
||
| // Metrics | ||
| sb.AppendLine("<table>"); | ||
| sb.AppendLine("<tr><th>Measurement</th><th>Value</th></tr>"); | ||
| sb.AppendLine($"<tr><td>Total number of PRs in range</td><td>{data.Metrics.TotalPRs}</td></tr>"); | ||
| sb.AppendLine($"<tr><td>Median: Hours to complete</td><td>{data.Metrics.MedianHoursToComplete:F1}</td></tr>"); | ||
| sb.AppendLine($"<tr><td>Percentage of PRs approved under 24 hrs</td><td>{data.Metrics.PercentApprovedUnder24h:F1}%</td></tr>"); | ||
| sb.AppendLine($"<tr><td>Percentage of PRs completed under 24 hrs</td><td>{data.Metrics.PercentMergedUnder24h:F1}%</td></tr>"); | ||
| sb.AppendLine("</table>"); | ||
|
|
||
| // Slow PRs | ||
| sb.AppendLine($"<h2>Long lived PRs (completed after 72 hrs): the past {data.WindowDays} days</h2>"); | ||
| if (data.SlowPRs.Count == 0) | ||
| { | ||
| sb.AppendLine("<p>🎉 All PRs completed within 72 hours this period!</p>"); | ||
| } | ||
| else | ||
| { | ||
| sb.AppendLine("<table>"); | ||
| sb.AppendLine("<tr><th>PR link</th><th>Hours to complete</th><th>Why so long?</th></tr>"); | ||
| foreach (var pr in data.SlowPRs) | ||
| sb.AppendLine($"<tr><td><a href=\"{pr.Url}\">{pr.Url}</a></td><td>{pr.HoursToMerge:F2}</td><td></td></tr>"); | ||
| sb.AppendLine("</table>"); | ||
|
|
||
| sb.AppendLine("<div class='key'>"); | ||
| sb.AppendLine("<strong>Why so long Key</strong>"); | ||
| foreach (var line in new[] | ||
| { | ||
| "💔 - Delayed getting reviews from PR Buddy™️ or Team", | ||
| "🤖 - Testing infrastructure, delayed intentionally", | ||
| "🌪️ - PR fell behind other priorities", | ||
| "⛱️ - Weekend/PTO delayed", | ||
| "🛠️ - Merging Blocked (ex. CI Failures, rule violations)", | ||
| "📜 - Lots of feedback", | ||
| "🙂 - No issues - just letting more people chime in", | ||
| }) | ||
| sb.AppendLine($"<p>{line}</p>"); | ||
| sb.AppendLine("</div>"); | ||
| } | ||
|
|
||
| // Appendix | ||
| sb.AppendLine($"<h2>Appendix — All PRs in Period ({data.AllPRs.Count})</h2>"); | ||
| sb.AppendLine("<table>"); | ||
| sb.AppendLine("<tr><th>PR</th><th>Title</th><th>Created (UTC)</th><th>Ready for Review</th><th>First Approved</th><th>Merged (UTC)</th><th>Duration</th></tr>"); | ||
| foreach (var pr in data.AllPRs) | ||
| { | ||
| var approved = pr.FirstApprovedAt.HasValue ? Ts(pr.FirstApprovedAt.Value) : "—"; | ||
| sb.AppendLine($"<tr><td><a href=\"{pr.Url}\">#{pr.Number}</a></td><td>{H(pr.Title)}</td><td>{Ts(pr.CreatedAt)}</td><td>{Ts(pr.EffectiveStart)}</td><td>{approved}</td><td>{Ts(pr.MergedAt)}</td><td>{FormatHours(pr.HoursToMerge)}</td></tr>"); | ||
| } | ||
|
Nigusu-Allehu marked this conversation as resolved.
|
||
| sb.AppendLine("</table>"); | ||
|
|
||
| sb.AppendLine("</body></html>"); | ||
|
|
||
| File.WriteAllText(outputPath, sb.ToString(), System.Text.Encoding.UTF8); | ||
| } | ||
|
|
||
| private static string Ts(DateTime dt) => dt.ToUniversalTime().ToString("MMM d HH:mm"); | ||
| private static string H(string s) => System.Net.WebUtility.HtmlEncode(s); | ||
| private static string FormatHours(double h) => | ||
| h < 24 ? $"{h:F1}h" : $"{(int)(h / 24)}d {(int)(h % 24)}h"; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| namespace NuGetDashboard; | ||
|
|
||
| public record RawPR(int Number, string Title, string Url, string Author, DateTime CreatedAt, DateTime MergedAt); | ||
|
|
||
| public record PRRecord( | ||
| int Number, string Title, string Url, string Author, | ||
| DateTime CreatedAt, | ||
| DateTime EffectiveStart, // ready_for_review → review_requested → created_at | ||
| DateTime MergedAt, | ||
| double HoursToMerge, // EffectiveStart → MergedAt | ||
| double? FirstApprovalHours, | ||
| DateTime? FirstApprovedAt); | ||
|
|
||
| public record DashboardMetrics( | ||
| int TotalPRs, | ||
| double MedianHoursToComplete, | ||
| double PercentApprovedUnder24h, | ||
| double PercentMergedUnder24h); | ||
|
|
||
| public record DashboardData( | ||
| string DateRange, | ||
| string AsOf, | ||
| int WindowDays, | ||
| DashboardMetrics Metrics, | ||
| List<PRRecord> SlowPRs, | ||
| List<PRRecord> AllPRs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callsNeeded = rawPRs.Count * 2underestimates actual API usage: the search itself can require multiple requests (pagination) and timeline/review endpoints may also paginate or require retries. This can cause the tool to proceed even when the remaining budget is insufficient. Consider making the estimate more conservative (include search pages + worst-case pagination) or degrade gracefully when the budget runs out mid-run.