https://github.com/WebKit/WebKit/pull/892 is an example of someone trying to contribute, but (I think) getting mixed up between the contents of the top message of the pull request and the ChangeLog. If there is reviewer information in the PR, we should be able to insert it into the commit message.
<rdar://problem/94256972>
I don't think that I'm following what happened. Maybe Jonathan will, but it just looks like the commit message doesn't follow our format - it's just this: >The source code and comment seem to have detached from each other, where the comment retains an old syntax. I don't think that automation should be able to re-format a sentence like this to a proper commit message. Not sure how the formatted commit message ended up in the top comment though.
> Not sure how the formatted commit message ended up in the top comment though. In this instance, while authoring the PR, I took a look at the other PRs while writing the PR body message but not the commit message itself. It could be more accessible for people to adjust their PR if the top comment was the basis of merges akin to squash merges. Thanks!
(In reply to Alexey Proskuryakov from comment #2) > it just looks like the commit message doesn't follow our format That's true. However, the top post in the PR thread _does_ follow our format. > > I don't think that automation should be able to re-format a sentence like > this to a proper commit message. > > Not sure how the formatted commit message ended up in the top comment though. The git commit message said this: ``` [WGSL] Adjust comment of test to match the tested source The source code and comment seem to have detached from each other, where the comment retains an old syntax. ``` The top post in the PR said this: ``` [WGSL] Adjust comment of test to match the tested source https://bugs.webkit.org/show_bug.cgi?id=240773 Reviewed by Myles C. Maxfield. * WGSLUnitTests/WGSLLexerTests.mm ``` And yet we failed to merge. Which is a bit contributor-hostile - they've told us everything we need to know to merge. They've even formatted it correctly! It's just in place B rather than in place A. We seem to just be insisting that the information come from place A rather than place B. We should allow the information to come from either place. The whole point of the move to GitHub is to make it easier for contributors to contribute. If a contributor goes through the trouble of formatting the entry but putting it in the top post in the PR instead of the commit message, we shouldn't punish them but should instead graciously accept the information they have given us - because they've given us all the information we need.
Yes, the suggestion is clear now. I'm still not sure about whether this is a one-off case, or something that many people will expect. Merging works on commits, and never pulls commit message from top comment in other GitHub projects AFAIK. So it seems like it would be super unusual, and could confuse people more than help them. Or are my facts wrong?
(In reply to Alexey Proskuryakov from comment #5) > Yes, the suggestion is clear now. I'm still not sure about whether this is a > one-off case, or something that many people will expect. > > Merging works on commits, and never pulls commit message from top comment in > other GitHub projects AFAIK. So it seems like it would be super unusual, and > could confuse people more than help them. Or are my facts wrong? Well, we already have code that knows how to parse ChangeLog entries. A first pass at this could be "if one parses as a ChangeLog entry and the other one doesn't, use the first one." (We could certainly do better in the future, and try to merge stuff together, but this simple first idea would have solved this issue.)
It is possible to construct a commit message from the PR description, but I'm a bit reluctant to do so at the moment. The reason this gets a bit complicated is that if we decide to require commit signatures (which is decently likely in the coming months), we can't modify commit messages as we land changes and have to decide where the merge-commit's message comes from. The PR description is a leading candidate for that, but we then have to decide where the PR description comes from in the first place, and it may not be the commit message.
(In reply to Jonathan Bedard from comment #7) > require commit signatures How will this be able to rewrite the commit message to change the "Reviewed by NOBODY (OOPS)" to "Reviewed by SomePerson." in that scenario?