[webkit-dev] ChangeLog Deprecation Plans

[Yusuke Suzuki]

> On Apr 18, 2022, at 5:38 PM, Michael Catanzaro <mcatanzaro at gnome.org> wrote:
> 
> 
> On Mon, Apr 18 2022 at 02:55:08 PM -0700, Yusuke Suzuki <ysuzuki at apple.com> wrote:
>> I think this is important. We are using commit message / ChangeLog as a document tied to the change, and we are writing very detailed description to make the intention / design of the change clear and making it as a good document when we read the change via git-blame, bisect, using that header, investigating how it works etc.
>> To make / keep this commit message / ChangeLog helpful, we need review, and I think reviewing of these messages is critical to keep usefulness of them.
> 
> I agree with all of the above. Actually, I'd suggest that the transition to git is a good opportunity to become a little stricter with commit message format than we historically have been. Except for trivial/obvious changes, more detail is better.
> 
> However, we don't need ChangeLog files in the repo or inline review comments to do this. I'm sure we can make do with the normal GitHub review UI.

GitHub (probably, Phabricator too) does not have UI to review commit message.
Also, showing commit message in an attached diff is pretty much similar to how Gerrit etc. shows the commit message for review. (Gerrit shows "Commit Message" diff).

> 
>> I think COMMIT_MESSAGE proposal has various good benefits.
>> 1. We can review commit mesasges.
>> 2. In local development, we can commit expected COMMIT_MESSAGE file directly in our tree. We can eventually add more and more detailed information to this file while local development continues, and we can also revert COMMIT_MESSAGE change since it can be commited in the local branch.
> 
> This has few advantages over using 'git commit --amend' or 'git rebase -i'. It would also be a custom, WebKit-specific part of our workflow. This is a good opportunity to remove as many WebKit-isms from our contribution process as possible, to make it easier for people who are not familiar with the project to contribute more easily. I suggest we at least try to do things like most other open source projects first, and only implement custom solutions if that fails.

Many projects are using multiple commits in one PR. Some projects require squashed merge (e.g. CoreCLR https://github.com/dotnet/coreclr/blob/master/Documentation/project-docs/contributing-workflow.md#suggested-workflow, https://github.blog/2016-04-01-squash-your-commits/ <https://github.blog/2016-04-01-squash-your-commits/>), and some projects just merge all of them. Because of the importance of performance / regression bisection, we must require landing a PR as one commit, so we need a way to squash commits.
If we would like to sane commit message for merged commits, we need to chose COMMIT_MESSAGE-like separated metadata for message or squashing manually.
Having separated metadata is much easier for development than manually squashing commits and messages because we can keep the final commit message safely even while changing history / code in this branch, and it was how webkit-patch worked.

> 
>> Commit message is tied to a commit, so editing commit without breaking commit-message is hard (how to revert one change from one commit without rewriting commit message? It requires some git hack). Separate independent COMMIT_MESSAGE file can solve this problem and makes local development easy.
> 
> Developers who are used to git -- which nowadays is pretty much everyone except WebKit devs -- are also used to rewriting commit messages.
> 
> Developers are NOT used to writing commit messages in a file named COMMIT_MESSAGE.

No. Various OSS projects require writing commit message separately from git-commit's message.
Chromium requires writing separate commit message data to upload a patch to Gerrit (git-cl).
And probably LLVM is too. (Using arc tool to upload a change to Phabricator).
They are conceptually the same to COMMIT_MESSAGE: managing non git-commit message for upstreaming.

Also, I'm fine if we can separately keep this message from git-commit. COMMIT_MESSAGE is one possible way.
But if we would like to have git-cl like tool which manages message separately from git-commit messages, then it is also fine. (but in that case, how to review this commit message is yet another problem).

> 
>> 3. This solves the problem how to squash multiple commits in one PR. Merge-queue can just look at this file and use this as a commit message when squashing and landing. This means that, in a PR, we can push multiple small commits in our PR branch.
> 
> What is the advantage to doing this, though? It's best if the commits in your PR match what you intend to land. The structure of commits is subject to review in most open source projects. If the commit history is messy, we should not approve the PR.
> 
> For now, we've agreed that for now each PR should land as one commit.

We do not need to repeatedly squashing commits just for review.

-Yusuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20220418/c3eed839/attachment.htm>