[webkit-dev] WebKit Transition to Git

[Manuel Rego Casasnovas]

On 14/10/2020 03:12, Ken Russell wrote:
> On Tue, Oct 13, 2020 at 2:37 PM Konstantin Tokarev <annulen at yandex.ru
> <mailto:annulen at yandex.ru>> wrote:
>     1. Workflow 1 - "Squash merge" policy
> 
>     * Whole PR is considered to be a single atomic change of WebKit
>     source tree. If work is supposed to be landed as a series of changes
>     which depend on each other (e.g. refactoring and feature based on
>     it, or individual separate features touching same parts of code),
>     each change needs a separate PR, and, as a consequence, only one of
>     them can be efficiently reviewed at the moment of time
>     * Commits in PR represent review iterations or intermediate
>     implementation progress
>     * Reviewers' comments are addressed by pushing new commits without
>     rewriting history, which works around GitHub's lack of "commit
>     revisions". Also this workflow has lower entry barrier for people
>     who haven't mastered git yet, as it requires only "git commit" and
>     "git push" without rebases.
> 
>     2. Workflow 2 - "Rebase" ("cherry-pick")) or "Merge" policy
> 
>     * PR is considered to be a series of atomic changes. If work
>     consists of several atomic changes, each commit represent an atomic
>     change
>     * Review iterations are done by fixing commits in place and
>     reuploading entire series using force push (of course if review
>     discovers that substantial part of work is missing it can be added
>     as a new atomic commit to the series)
>     * It's possible to review each commit in the series separately
>     * Workflow requires developers to have more discipline and
>     experience with using git rebase for history rewriting. Entry
>     barrier can be lowered by providing step by step instructions like
>     e.g. [1].
> 
> 
>     Workflow 1 is more like what we have now with Bugzilla.
> 
>     Workflow 2 is used by many projects which use git for a long time
>     and value high-quality commit history. It's used e.g. by Linux
>     kernel, or projects which use Gerrit as a review tool (Chromium,
>     Android, Qt, etc). It advantages for developers (who can submit more
>     work to review at the same time without risk of mixing up unrelated
>     changes together), reviewers (it's easier to review atomic changes
>     than those with too high or too low granularity), maintainers of
>     stable branches (they can pick bug reports and avoid at least some
>     unneeded refactorings, features and style improvements).
> 
> 
> Slight correction: Chromium's Gerrit instance actually uses Workflow 1 -
> the "squash merge" policy. The developer can have multiple Git commits
> on a single branch and associated with a single Gerrit code review. Each
> time the branch is uploaded into the review tool, all of those commits'
> diffs are squashed together into the current patchset - the new version
> of the review.

Yeah this Workflow 1 has been working fine in Chromium since the fork,
and it's pretty similar to what we currently have in WebKit bugzila, so
I think it'd be the best option when WebKit is on Git too.

If I understood correctly, the only difference with Chromium's Gerrit
would be that you cannot mark dependencies between PRs, but that's not
something that people use every day (mostly is used when you want to
test your patches in advance against the tryjobs/EWS or the perfbots)
and as Ryosuke mentioned you could share that WIP commits in your own
branch, or even in a dummy PR if you need to check EWS results and
discard that afterwards once it's split in several PRs.

I also don't see the problem with creating a new bug entry for a small
change and make the change separately, I have asked for that kind of
things in WebKit bugzilla a few times when I reviewed a patch that had
some unrelated change that could be landed separately. I know that's not
always the case and sometimes we land bigger patches than it should, but
I think that's more on the author and reviewer considerations.

My 2 cents,
  Rego