Skip to content

internal/issues: remove author CC from Github issue #72244

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

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

mgartner
Copy link
Collaborator

Previously, automatically created issues would CC the author of the
test. In many cases, tests can fail for reasons outside of the author's
domain of knowledge, and incorrect CC's create unnecessary noise.
Mentioning the team that owns a test is sufficient for routing an issue
to the most appropriate subset of engineers.

Release note: None

@mgartner mgartner requested review from a team and erikgrinaker and removed request for a team October 29, 2021 18:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member

tbg commented Oct 29, 2021

The author is not pinged. It is only written into the commit message to facilitate manual routing, if necessary. Are you seeing something different? I'm not generally opposed to removing the author entirely, but the goals of the PR should already be achieved today without making any change.

@mgartner
Copy link
Collaborator Author

Are there cases where manual routing to the author is necessary if the team is already pinged?

@tbg
Copy link
Member

tbg commented Oct 29, 2021

Hard for me to speak for everyone, but at least on KV pings are not disseminated to all members (too noisy). So we do rely on git blame for routing occasionally. I would think that by continuing to include this information in the issue we are doing no harm (since the person is not notified; as far as Github is concerned they are not mentioned). If we remove it, we could be annoying someone.

What is maybe another argument for the removal, though, is that it's just such a weird one-off. I think I have seen folks think that there was a bug in the issue poster and that it was accidentally not mentioning the author despite putting it in the issue.

I'm fine removing this.

@mgartner
Copy link
Collaborator Author

I would think that by continuing to include this information in the issue we are doing no harm

The motivation I didn't mention for this (because it's subjective) is that calling out specific people seems counter-productive for team cohesion.

But I don't want to break people's workflows with this change. I'll close for now.

@mgartner mgartner closed this Oct 29, 2021
@mgartner mgartner deleted the remove-author-cc branch October 29, 2021 19:55
@mgartner mgartner restored the remove-author-cc branch October 29, 2021 20:06
@mgartner mgartner reopened this Oct 29, 2021
Previously, automatically created issues would CC the author of the
test. In many cases, tests can fail for reasons outside of the author's
domain of knowledge, and incorrect CC's create unnecessary noise.
Mentioning the team that owns a test is sufficient for routing an issue
to the most appropriate subset of engineers.

Release note: None
@mgartner mgartner requested a review from a team as a code owner October 29, 2021 20:10
@tbg
Copy link
Member

tbg commented Oct 29, 2021

(We talked it over and we think it's time for this half-mention to go; if someone violently objects it's easy to revert)

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

I'm fine with removing this. Seems like a team responsibility, and if it's unclear who to route to then it's easy enough to look at the blame.

@mgartner
Copy link
Collaborator Author

mgartner commented Nov 1, 2021

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 1, 2021

Build succeeded:

@craig craig bot merged commit da7b0c8 into cockroachdb:master Nov 1, 2021
@mgartner mgartner deleted the remove-author-cc branch November 1, 2021 13:10
tbg added a commit to tbg/cockroach that referenced this pull request Nov 11, 2021
We completely removed any functionality attached to this in cockroachdb#72244.

Release note: None
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