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

[meta] Enforce scoped Rust-GCC/gccrs#NNN instead of unadorned #NNN in Git commit logs #3213

Open
tschwinge opened this issue Oct 23, 2024 · 1 comment
Labels

Comments

@tschwinge
Copy link
Member

I had wanted to bring this up for a long time, but... As is usually good practice (no question about that!), GCC/Rust Git commit logs sometimes use #NNN to refer to GitHub GCC/Rust Issue or Pull Request #NNN. However, in upstream GCC, #NNN have a very specific meaning: to refer to GCC Bugzilla #NNN, so it's very confusing if GCC/Rust commits get upstreamed, where their Git commit logs contain #NNN, which don't refer to GCC Bugzilla, but rather to GCC/Rust Issue or Pull Request #NNN. That means, for example, that GCC/Rust commit logs' #NNNs in several occasions got auto-linked to (totally unrelated!) upstream GCC bugs #NNN.

In context of the "Sourceware Forgejo experiment", @jwakely again raised this in https://inbox.sourceware.org/CAH6eHdTnB5Bqru=mZNzXmyu29D8PwCKj5Z1mbaAfA7rbrrDrYg@mail.gmail.com "Linking commits to PRs/MRs":

We have hundreds, if not thousands, of
commits where the message refers to a Bugzilla bug as #1234 [...]
We should ask the gccrs devs to
stop using Issue #2514 to refer to issues in their external github repo.
They could use Rust-GCC/gccrs#1234 instead, which won't refer to some
arbitrary #1234 in an unrelated repo. e.g. for gccgo the commits include
things like "Fixes golang/go#33739" which is fine, the ID is scoped so
won't link to the wrong thing.

(I've added a few back-ticks to Jonathan's text, in order to stop auto-linking for those, here. ;-)

See https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls.

This checking would (preferrably) go into .github/workflows/commit-format.yml (right?), and/or be verified when upstreaming commits.

It should highlight (or reject?) any unadorned #N, PRNNN, PR COMPONENT/NNN. (Have to check the exact patterns that GCC upstream considers to "own".) Probably is has to be "highlight" instead of generally "reject": there may be valid cases where GCC/Rust PRs contain Git commits with commit logs containing #NNN, such as, when cherry-picking commits from GCC upstream.

@CohenArthur
Copy link
Member

I think this is a great idea. My thinking is that "reject" but can be overwritten if it is a legitimate use of the #NNN tag (which does happen but quite rarely I think - when fixing bugzilla bugs I always end up linking to the full PR...). so I like the idea of having a CI step check that for us, but it needs to not be a requirement for merging - this way we can still merge if that step fails

@CohenArthur CohenArthur added the CI label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants