Forbidding @username in commits

There have been a few instances where others have made commits that tag me using @username in commits made to the LLVM repository. This has the side-effect of notifying me for each of those commits being pushed, not just the PR changes. That’s somewhat manageable for the lifetime of the PR, but it doesn’t stop once the PR is merged. Every time that commit gets pushed to a fork, or cherry-picked to a fork, or any other action that results in a commit being pushed to GitHub that contains that commit message in some form, I get an email about it, and that can have a very long tail.

I would therefore like to propose a policy of not using the @username syntax in commit messages. Unprefixed usernames, real names, names + email addresses and any other sensible way of identifying individuals would remain permitted, this is solely forbidding the specific syntax that triggers GitHub (and perhaps other forges hosting LLVM forks, if there happens to be a user there with the same username, whether the same person or not) to send an email.

13 Likes

At least you’ll always know what that commit is up to. :smiley:

Enforcing this would have to be automatic to be effective. Accidentally leaving a tag in is too easy to just rely on a policy.

Ideally. But, assuming it’s going via a PR that isn’t instantly merged, you are going to get a notification in your inbox that you can use to tell the author to change the message.

I have mixed feeling on this. On one hand the switch to github greatly increased my e-mail volume For me it has been a poor tradeoff.

On the other hand, this very much feels like a github problem. They should have the tooling in place for you to easily silence notifications on PRs and other things etc. In general I feel like their tooling around reducing over communications is pretty bad and we should really pushing back on github and not tailoring our workflows to work around their poor product designs.

I don’t have illusions on the effectiveness of trying to get action from github here.

3 Likes

Not that I’ve had this happen to myself that I can think of, but I see no reason to permit the notation. In a person’s local git clone, the “@” character is just noise, so it shouldn’t be present in the PR description (which becomes the commit message) in the first place. If people need pinging, they can be pinged via the @ notation in a follow-up comment.

2 Likes

While automation probably makes sense, I think there is already value in having something like “Please don’t use the @<username> syntax in commit messages; use <username>/<Full Name> (or Github’s Co-authored-by where adequate).” in the developer policy, e.g., in the “Attribution of Changes” section.
I certainly wouldn’t have brought this bane to @jrtc27 (sorry for that!) if I had known that GitHub handles mentions in commits this way.

1 Like

Agree that we should document this, and ideally have automation to notify the PR author about any mentions.

Of course, the fact that mentions in commit messages are a thing at all is a plain bug in GitHub, but it’s not like having to work around GitHub deficiencies is a new problem…

4 Likes

I deliberately didn’t name names, and this isn’t the only case of it (nor the only project where I’ve seen it happen)

1 Like

I agree that this is a Github problem but that we should be pragmatic. I’m supportive of adding it to the developer policy and discouraging it.

Assuming this doesn’t happen all that frequently, and doesn’t become more prevalent, I think automation would be a nice-to-have but not essential. If I’m being tagged there’s a good chance I’ll see the PR before it’s merged. There are a bunch of things that happen way more frequently that aren’t automated and for which I manually point folks to the LLVM developer guide.

I’ve had to block someone on GitHub because I couldn’t work out how to stop receiving emails every time they pushed their (presumably rebased) local branch to GitHub.

I support adopting this policy, and agree that we should have some form of tooling to catch the issue. Those don’t need to happen in lockstep.

1 Like

I support adopting this policy, and agree that we should have some form of tooling to catch the issue. Those don’t need to happen in lockstep.

+1