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

cc @rust-lang/wg-compiler-performance on master perf regressions. #1369

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

nnethercote
Copy link
Contributor

To get more visibility on regressions, and earlier.

To get more visibility on regressions, and earlier.
@Mark-Simulacrum
Copy link
Member

I'm not entirely opposed, though I do worry a little about diffusing responsibility - might be seen as a sign that folks don't need to do anything if we've already been pinged. Not sure how to best balance that.

@nnethercote
Copy link
Contributor Author

Previous complaints about the github comment have included:

  • it's too long
  • people don't read it
  • if they read it, they don't understand it, and/or don't take action

But now the concern is about a potential person who does read and understand the message, and would have taken action, but the addition of a single "cc" at the bottom causes them to behave differently? I consider that unlikely.

In contrast to this possible effect, there will be a definite effect: several interested people will get CC'd immediately on all regressions. This makes it much more likely that something will happen before weekly triage, making life easier for triagers.

@Mark-Simulacrum
Copy link
Member

Well, let's try it. As I mentioned on the other PR, I'm generally happy to experiment here, though would love to put some work into tracking effectiveness of our comment strategy (probably along "author/reviewer do some triage work" lines).

@Mark-Simulacrum Mark-Simulacrum merged commit f365f30 into rust-lang:master Jul 29, 2022
@nnethercote nnethercote deleted the gh-comments-1 branch July 29, 2022 19:48
@nnethercote
Copy link
Contributor Author

rust-lang/rust#99123 is the first example of this change having an effect. I never would have seen that PR previously :)

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