[summary] Make PR comment pretty #2946
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
2 times, most recently
from
ebea4ad to
205372f
Compare
Performance Summary for commit 1cb21b6CI Runtime ComparisonWorkflow: Rust
|
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
2 times, most recently
from
02c067b to
f4d0da9
Compare
Performance Summary for commit 1cb21b6CI Runtime ComparisonWorkflow: Rust
|
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
from
f4d0da9 to
05bd4fd
Compare
Performance Summary for commit 1cb21b6CI Runtime ComparisonWorkflow: Rust
|
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
from
05bd4fd to
324dd57
Compare
ndr-ds
force-pushed
the
11-16-track_ci_job_runtime
branch
from
fceadb0 to
1870f37
Compare
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
from
324dd57 to
96d32d0
Compare
ndr-ds
force-pushed
the
11-16-track_ci_job_runtime
branch
from
1870f37 to
a988c06
Compare
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
2 times, most recently
from
1599423 to
79ac932
Compare
jvff
approved these changes
Nov 25, 2024
| } | ||
|
|
||
| // The key is the name of the workflow, and the value is a list of per job comparisons. | ||
| #[derive(Serialize)] | ||
| pub struct CiRuntimeComparison(HashMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>); | ||
| pub struct CiRuntimeComparison(pub HashMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>); |
There was a problem hiding this comment.
Optional idea: we could sort the workflow results by name (will probably require updates to the import and to the collect below).
Suggested change
| pub struct CiRuntimeComparison(pub HashMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>); | |
| pub struct CiRuntimeComparison(pub BTreeMap<WorkflowName, Vec<WorkflowJobRuntimeComparison>>); |
ndr-ds
changed the base branch from
11-16-track_ci_job_runtime
to
graphite-base/2946
ndr-ds
added a commit
that referenced
this pull request
Nov 26, 2024
## Motivation It would be nice to see CI runtime regressions or improvements in the PRs. ## Proposal Create Rust CLI tool that checks runtimes, compares against base ref, and comments on the PR with a summary of the changes. This works, but as you guys can see it's pretty ugly for now, we're just posting the JSON 😅 We're also commenting for every successful workflow run in CI. The PR is already pretty big so I will do the following follow ups: - [x] Markdown format this in a pretty way (done in #2946) - [x] Add the performance summary markdown as a [job summary](https://github.blog/news-insights/product-news/supercharging-github-actions-with-job-summaries/) of the job, so people have access to historical versions of this (done in #2948) - [x] Find if there's already a performance summary comment, and edit it instead of always posting a new one (done in #2953) - [ ] Might make sense to only post if the change exceeds some percentage threshold for now. Open to suggestions Other follow ups are tracked in the issue for this: #2834 ## Test Plan Triggered a modified version of this manually with workflow dispatch, it works, but we can unfortunately only know for sure this works by merging :/ ## Release Plan - Nothing to do / These changes follow the usual release cycle.
ndr-ds
force-pushed
the
11-22-_summary_make_pr_comment_pretty
branch
from
d4718b5 to
666a16b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Right now we're only commenting the JSON of the results, we can probably do better
Proposal
Comment a formatted Markdown of the results
Test Plan
Since #2936 is not merged yet, I have to test this in the same way I tested #2936
Release Plan