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

Soften the language of regression comments on merged PRs #1923

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Jun 11, 2024

It has been noted several times on Zulip that the current message on regressions is perhaps too aggressive, and does not help contributors enough. It also does not mention the possibility that the found regression is just a false alarm, which does sadly happen quite often.

This PR is an attempt to improve upon that. Let the bike-shedding commence!

@workingjubilee
Copy link
Member

hello @workingjubilee please remember to get to this tomorrow ok?

@workingjubilee
Copy link
Member

damn I can't notify myself.

@lqd
Copy link
Member

lqd commented Oct 3, 2024

I've also discussed this topic with contributors at rustconf, and some piece of feedback I received was that the noise in the system and false positives that can happen, may also warrant a reword of the very-confident "This is a highly reliable metric" message. This has caused people to investigate regressions that weren't there, but didn't know about these annoying occurrences.

This PR already mentions the noise so we may just need to soften the metrics descriptions and fix this at the same time.

@Kobzol Kobzol force-pushed the regression-pr-comment-change-text branch from 21490de to 13a4495 Compare October 4, 2024 07:21
@Kobzol
Copy link
Contributor Author

Kobzol commented Oct 4, 2024

Tried to reword in 13a4495.

@nnethercote
Copy link
Contributor

lgtm!

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Yes, this looks much better. And it can always be tweaked further while trying to walk the line between "eye-catching enough to direct attention" and "not directing that attention incorrectly".

@Kobzol Kobzol merged commit de7ce9e into rust-lang:master Oct 7, 2024
11 checks passed
@Kobzol Kobzol deleted the regression-pr-comment-change-text branch October 7, 2024 19:28
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.

4 participants