On 02/10/2020 18:43, Jonathan Bedard wrote: Hi all, This is great to hear! Several people have already made comments about things that I wanted to say, but I'd just to add my personal point of view...
I've used both Mozilla's Phabricator and Chromium's Gerrit for patches/review, and I find both of them great and pretty easy to use. Of course it can take time to get familiar with the UI, but I don't think this is a big blocker, the same is true with any tool... Mozilla and Chromium projects are basically using a separate tracker for bug reports and a Phabricator's differential / Gerrit's CL are connected to a bug entry. Mozilla's differential are attached to Bugzilla as a patch (which is good to make people cc'ed to the bug aware of WIP patch) while Chromium's Gerrit only posts the commit message when the patch lands. Both tools generate a increasing ID in the commit message. However, I think Phabricator's ID is done when the differential is created, not at commit time so this is not what we want for WebKit I guess. In any case, I don't think there should be a problem to add a hook to ensure the ID is created when landing the patch. When you upload a patch you are basically creating a new differential/CL or updating an existing one with a new revision and the difference between revision is calculated automatically. To upload a patch, you execute a script similar to our "webkit-patch upload" which can do extra checks (e.g. style, suggesting reviewers, etc). In any case, Mozilla and Chromium's projects perform these checks when the patch is uploaded too. I'm not sure for Mozilla's phabricator but in Chromium's Gerrit you can also make a CL depend on another one (I mean the review tool and bots will understand that dependency and behave "nicely"), which is helpful to split a change into several patches. I personally prefer both of them over GitHub/GitLab's approach with branches & merge/pull requests. That way you don't have to publish your branches / rebases / forced pushes / squashes, etc Instead, you would do whatever you want in your local clone & branches and always only send the relevant diff as a new revision. Others say similar features are available/planned for GitHub/GitLab, so maybe it's not a problem. One thing to take into account is that WebKit's repository is big and public GitHub/GitLab prevent creating large repository by default. This means it might not be possible for contributors to actually fork WebKit's repository on their account and then create a pull request (which is the standard way IIUC). Instead, we would probably end up doing like web-platform-tests and give contributors the permission to create branches to the WebKit account and make Pull Request to the master branch. Probably, we should forbid people to commit to the master branch directly (I think someone broke WPT's master branch that way last year)... Similarly, public GitHub/GitLab prevent very large files by default. Hopefully, this does not happen in WebKit (we should definitely add a hook to prevent someone to land a big file) but for example that mistake happened in Chromium last year ( https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/tSOmIfXAP2s/m/H07SmWIoCgAJ ) and this broke the sync of their GitHub mirror. I don't know the details of the conversation between Google & GitHub, but it took several weeks and at the end at Igalia we decided to switch our MathML development branch to an internal GitLab repository on which we have full control. Apple might want to weight the pros/cons of managing their own instance VS relying on proprietary / third-party instances for this kind of issue or other customizations. In general, I personally believe it would be good if one only require to use free software tool to contribute to WebKit. But I guess we already made an exception by switching to Slack, so... -- Frédéric Wang _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
