Skip to content
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

Show mean of primary/secondary results in GitHub summary comment tooltip #1907

Merged
merged 1 commit into from
May 14, 2024

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented May 14, 2024

The performance summary tables in GH comments (e.g. this one) show all metrics other than instruction count in a collapsed manner. I think that this sometimes leads to missing interesting information in these tables, if no one clicks on them.

This PR adds a short summary of the performance result to the Results reveal button, so that it is clear how large is the effect even without clicking on the button. It should look like this:

image

@Kobzol Kobzol requested a review from nnethercote May 14, 2024 09:08
Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tooltip seems like a strange name. To me that means something that shows up when you hover over a GUI element. Maybe call it summary? Otherwise, LGTM.

@Kobzol Kobzol force-pushed the gh-summary-tooltip branch from 9909bdc to a316e6d Compare May 14, 2024 10:26
@Kobzol
Copy link
Contributor Author

Kobzol commented May 14, 2024

I also didn't like the name, but couldn't think of a better one. Summary is already kind of overloaded in the code, but it is a good name nevertheless. Changed the PR to use it.

@Kobzol Kobzol enabled auto-merge May 14, 2024 10:27
@Kobzol Kobzol merged commit f520cec into rust-lang:master May 14, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants