Conversation
Edited 2 months ago

i think that GitHub has firmly established some unnecessary preconceptions about how code review should work

if you look at the code forges that came after (GitLab, Gitea), most of them copy the Pull Request interface because it's familiar and easy to use

even though PRs as a concept exist with email-based workflows, [0] these modern code forges assume that PRs are the finest granularity you are working at. there are no accommodations for atomic commits, despite some of Git's tools being designed for this. [1]

having used it over the summer, i do think that gets it right. but nobody wants to copy Gerrit when making a comprehensive forge. the closest thing we have is (uses email for submitting patches), which is not really what i want.

as it stands, Gerrit appears to be super underutilized in open-source spaces. the only project i'm aware of (that isn't being operated by a company) that uses it is by @hikari. [2] aside from the friction of setting Gerrit up to work with a more featureful forge, i think the reason for this may be a resistance to more bureaucracy when working on fun stuff.

but like. reviewing GitHub PRs with more than one commit kinda sucks. yet you don't want to go full Gerrit (+cgit) either, because Gerrit's pipeline integration appears to be pretty primitive. also, it's nice to have a repo browser other than cgit

i think that my ideal setup would be a federated (!) code forge with gerrit-style code review of individual commits and modern CI integration

[0] https://git-scm.com/docs/git-request-pull
[1] https://gerrit-review.googlesource.com/Documentation/intro-rockstar.html
[2] https://github.com/touchHLE/touchHLE

1
0
0

MatΔ›j Cepl πŸ‡ͺπŸ‡Ί πŸ‡¨πŸ‡Ώ πŸ‡ΊπŸ‡¦

@koopa @hikari

https://review.opendev.org/ is for , but my personal experience is that Gerrit is better in theory than in practice. I had to use it for some internal tools in for some time (a long time ago) and the experience was absolutely horrible (see https://youtu.be/L8OOzaqS37s?t=485 … @gregkh is much smarter than me). I agree with you on the rest, GH-style PRs are actually quite unpleasant when once gets used to alternatives, I really like and yes is awesome.

2
0
0

MatΔ›j Cepl πŸ‡ͺπŸ‡Ί πŸ‡¨πŸ‡Ώ πŸ‡ΊπŸ‡¦

Edited 1 month ago
0
0
0

@mcepl @koopa @gregkh I should take some time later to watch that and see what they didn't like about Gerrit. I have a feeling it might be because it's kind of corporate-oriented rather than open-source-project oriented. touchHLE manages to make it work but we're a tiny project and we have a semi-closed development model (i.e. me and all the other contributors are in a chat channel and can co-ordinate behind-the-scenes). it might not work well for a fully open project because it makes it easy to swamp a maintainer with work. but i feel like the code review model in it is the right one and it mostly just needs refining on the social side, if that makes sense.

2
0
0

@mcepl @koopa @gregkh you won't be surprised to learn I was first exposed to Gerrit in a corporate environment. it is very well-suited to that. touchHLE was initially all GitHub but GitHub sucks for large numbers of small patches, and I knew Gerrit would make that more manageable

1
0
0

@mcepl @koopa @gregkh oh I'm actually quite confused about the comment that patch series are something Gerrit fails at. in my experience its defining feature is that it handles related change-sets exceptionally well. but this requires the unique-ID-in-commit-message hook

3
0
0

MatΔ›j Cepl πŸ‡ͺπŸ‡Ί πŸ‡¨πŸ‡Ώ πŸ‡ΊπŸ‡¦

@hikari @koopa @gregkh

No, no, his (and mine) complaints were completely based on the technical merrits.

0
0
0

@mcepl @koopa @gregkh parting thought (sorry Greg, will untag after this): Gerrit is essentially an attempt to take the LKML-style patch series email workflow, but putting it into a git-first database-driven webapp, with all that entails. all the pros and cons flow from that.

1
0
0
@hikari @mcepl @koopa Agreed, and for smaller projects, gerrit is almost even nice to use, but for ones with any "real" amount of size of developers and rate of change, it is rough and I curse it every time I have to review a kernel patch series in the Android gerrit instance :)
1
0
2

MatΔ›j Cepl πŸ‡ͺπŸ‡Ί πŸ‡¨πŸ‡Ώ πŸ‡ΊπŸ‡¦

Edited 1 month ago

@hikari @koopa @gregkh

And what I remember it is very very easy to mess out Change-IDs and you have to redo everything again. Or what about rebasing the patch series on the top of changed master which includes some commits from the series? Or multiple series stacked one on the top of each other (the thing which now even plain git does reasonably well with `git rebase --update-refs`)? There were so many ways how to break it, it was almost constantly broken.

1
0
0

@mcepl @koopa @gregkh IME there is a learning curve to it but it can handle all of these very well once you're used to it, but it's a very different model from conventional git use, so it takes a bit to fully grok it

0
0
0

@mcepl @koopa @gregkh I accidentally severed part of this thread from my instance's perspective, gonna link it here so others can follow https://social.kernel.org/objects/74412973-c5de-4b24-b73b-2f0c817026c9

0
0
0

@gregkh @mcepl @koopa one interesting advantage of Gerrit is it can handle medium-size binary files. badly, of course; they're basically unreviewable in any git-ish system, but at least they don't have to go through email. how does Linux manage to deal with that?

1
0
0
@hikari @mcepl @koopa Linux doesn't have binary files in its source tree (for the most part, there are a few tiny exceptions), so this isn't an issue at all.
1
0
1

@gregkh oh, how do you handle the firmware blobs and stuff?

1
0
0
@hikari What firmware blobs? The old ones were all in text format anyway (arrays of bytes in a .c or .h file), so that's never really been an issue. I think they are almost all gone from the tree now so that shouldn't be an issue.

For the linux-firmware project, you can send binary diffs to them, in email, git handles that just fine. You aren't reviewing a binary file anyway, it's either you take it or you don't.
1
0
2

@gregkh I see, thanks. I guess linux-firmware is what I was referring to, I wasn't sure if it was a separate tree.

0
0
0