[webkit-dev] WebKit Transition to Git

[Adrian Perez de Castro]

Hello,

On Fri, 02 Oct 2020 13:48:56 -0700 Ken Russell <kbr at google.com> wrote:

> Excited about this migration! One comment inline.
> 
> On Fri, Oct 2, 2020 at 9:43 AM Jonathan Bedard <jbedard at apple.com> wrote:
> 
> > *Patches to Pull Requests*
> > In the coming months, more automation will start using the git version of
> > the repository instead of the Subversion version. Even immediately after we
> > switch, the patch review workflow will remain what it is now (EWS bots
> > mostly already use a git clone as their checkout). We do, however, intend
> > to switch from a system of patch review to a system of pull requests. This
> > process will be incremental and the patch review system will remain alive
> > as long as Bugzilla does.
> >
> > *GitHub Issues*
> > The last part of transitioning away from Subversion is to re-evaluate our
> > bug tracker. Bugzilla has served us well, but seems to be an impediment to
> > engaging with the open source community. We are interested in moving away
> > from Bugzilla and to GitHub Issues, allowing new developers to work with a
> > system they are likely already familiar with and to allow us closer
> > integration with repositories managed by W3C. Although GitHub Issues has
> > had performance problems in the past with projects that have many bugs,
> > these performance problems have been resolved to the satisfaction of a few
> > large projects hosted on GitHub that are of a comparable scale to WebKit.
> > The biggest blocker we are aware of is managing security bugs, since the
> > security advisory system used by GitHub is essentially the opposite of how
> > WebKit security bugs work. Moving to GitHub Issues, if it happens, will be
> > the last part of this transition, and we are interested in soliciting
> > feedback from our contributors on what the WebKit project’s integration
> > with GitHub Issues should look like.
> 
> Github's code review UI has a couple of feature gaps in my opinion. It's
> difficult to look at earlier versions of the pull request, in particular to
> verify that issues found during code review have been fixed. I remember it
> also being difficult to figure out whether all comments on earlier versions
> have been addressed.

There is also one annoyance which has been slowly grinding my gears: the
GitHub diff viewer (including in reviews) hides “big diffs” for some arbitrary
and unknown definition of “big”, forcing a reviewer to manually click in a
“Load diff” link, for each hidden diff, every time. In a few cases I ran into
“very big” diffs or diff hunks which could not even be displayed that way and
had to fetch the branch locally to review the changes. Truth be told: it has
been a good while since I haven't found any diff which could not be loaded
at all, so maybe that issue has been fixed by now.

Now, GitLab's diff viewer has its own set of similar-but-not-quite-same
annoyances. Instead of hiding “big” diff hunks, it hides “random” hunks:
typically big ones, but sometimes small ones as well. A positive point for
GitLab is that at least they can always be loaded using that pesky “Load
diff” link.

Why none of Git{Hub,Lab} would have an option “gimme all the diffs” user
option, or at least remember which reviews I visited before where the “Load
diff” links have been used is something inexplicable. Or they could just
show the diffs ¯\_(ツ)_/¯ 

> Gerrit code review has worked well in my experience. It explicitly manages
> different versions of the same patch set, and prominently shows whether all
> code review comments have been addressed.

One point for Gerrit: I never ran into a diff that it would not show.

Regards,
—Adrián
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20201008/33bcaa18/attachment.bin>