Bug 239328

Summary: Do not require writing commit message
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: bfulgham, cdumez, jbedard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 239082    

Description Yusuke Suzuki 2022-04-14 03:01:47 PDT
Commit message should be generated from ChangeLog automatically.
Currently, we need to write ChangeLog and commit messages. So, it requires writing it twice.
And commit message is fragile: if we are working on local branch with multiple small changes, canonical commit message requires us to squash local commits for PR, which destroys the existing workflow (like https://bugs.webkit.org/show_bug.cgi?id=239327), which was working with webkit-patch.

So, I think, at least, we should avoid writing the same things twice (ChangeLogs and commit message).
Comment 1 Yusuke Suzuki 2022-04-14 03:06:32 PDT
I have two ideas.

1.
If we keep ChangeLogs, then I think `git webkit pr` should create PR-specific branch and generate commit messages from ChangeLogs automatically, so PR is always one commit, and we do not need to reflect our local branch's way (how many commits the local branch has etc.), and we can avoid writing the same ChangeLogs twice.

2.
If we drop ChangeLogs soon, then I suggest adding COMMIT_MESSAGE file instead. It will not be landed in the tree. And locally, we can commit in our local development branch. And git webkit generates this file automatically as we do for ChangeLogs.
We modify it for messages. git-webkit pr creates separate PR branch, which uses COMMIT_MESSAGE as a commit message and PR's issue message. But that branch also includes COMMIT_MESSAGE as a change. So reviewer can comment on commit log messages.
And when merge-queue / git-webkit lands the change, it remove COMMIT_MESSAGE from the change, and use that content as a commit message.
Comment 2 Yusuke Suzuki 2022-04-14 03:07:58 PDT
(In reply to Yusuke Suzuki from comment #1)
> 2.
> If we drop ChangeLogs soon, then I suggest adding COMMIT_MESSAGE file
> instead. It will not be landed in the tree. And locally, we can commit in
> our local development branch. And git webkit generates this file
> automatically as we do for ChangeLogs.
> We modify it for messages. git-webkit pr creates separate PR branch, which
> uses COMMIT_MESSAGE as a commit message and PR's issue message. But that
> branch also includes COMMIT_MESSAGE as a change. So reviewer can comment on
> commit log messages.
> And when merge-queue / git-webkit lands the change, it remove COMMIT_MESSAGE
> from the change, and use that content as a commit message.

I think (2)'s way is how Gerrit etc. is doing. For example,
https://chromium-review.googlesource.com/c/chromiumos/chromite/+/3586278
File "Commit message" is included in the change.
Comment 3 Jonathan Bedard 2022-04-14 08:01:55 PDT
There is a 3rd option: get rid of ChangeLogs and exclusively rely on commit messages.

I would prefer option #2, with a fallback to #3 if no COMMIT_MESSAGE is found. Note that Merge-Queue already modifies commits, and the difficulties of automatic squashing is all about commit messages, a COMMIT_MESSAGE file would fix that problem.
Comment 4 Yusuke Suzuki 2022-04-14 17:40:53 PDT
I think this is a release blocker level issue because various PRs are already failing to include ChangeLogs https://github.com/WebKit/WebKit/pull/291 https://github.com/WebKit/WebKit/pull/286. I think that this must be fixed before officially switching to GitHub.
So far, it looks like 1-3 options work well, so some of them need to be implemented :)
Comment 5 Alexey Proskuryakov 2022-04-14 18:02:48 PDT
I like option 3. I think that we should follow common GitHub workflows, even at the cost of regressing convenience in some cases. We shouldn't make it look like Gerrit or Subversion or anything else unless absolutely necessary (as it is with monotonically growing revision numbers).
Comment 6 Yusuke Suzuki 2022-04-14 18:30:03 PDT
(In reply to Alexey Proskuryakov from comment #5)
> I like option 3. I think that we should follow common GitHub workflows, even
> at the cost of regressing convenience in some cases. We shouldn't make it
> look like Gerrit or Subversion or anything else unless absolutely necessary
> (as it is with monotonically growing revision numbers).

With option (3), how can we create multiple commits locally?
Comment 7 Radar WebKit Bug Importer 2022-04-21 03:02:13 PDT
<rdar://problem/92084272>
Comment 8 Brent Fulgham 2022-06-23 16:37:23 PDT
This is no longer an issue now that we have dropped ChangeLogs.