[AaronFriel]

To be polite, I think the target audience of these tools might not include you. While that workflow works for you and, apparently, scales for some very large projects like the Linux kernel, it isn't a good solution for an enormous number of people which is why tools like Github, Gerrit, GitLab, and others exist.

[u801e]

> To be polite, I think the target audience of tools might not include you.

Insinuating that I'm a special case doesn't add to the discussion.

[AaronFriel]

You aren't a special case, but if you don't see any flaws with your method of code review then I don't think you're the target market for code review tools.

[h0l0cube]

I have no dog in this fight, but you stated:

> it isn't a good solution for an enormous number of people

.. without providing any insight or justification for that belief. So an interesting and productive tangent might be to elucidate what you believe those flaws are?

Unpacking this question might even lead to some good UX ideas that can be applied to today's review systems?

[fireattack]

I would provide one from someone who is not a dev or SE.

I honestly never fully understand how to read the maillist patch. The plain text format makes it very hard to understand what's going on. I'm sure I will understand it better or even prefer it if I use it long enough, but then again I can instantly understand code reviews on GitHub/GitLab/Gerrit.

[h0l0cube]

I've never had to use a maillist patch myself, but from the ones that I've glanced at, I have this same problem, that formatting, syntax highlighting is absent. It seems like this would be an easy problem to fix by using a better viewer, so that might be enough to make it comprehensible.

[u801e]

> I honestly never fully understand how to read the maillist patch. The plain text format makes it very hard to understand what's going on.

That may be due to the settings in your mail client. If it's displaying plain text in a variable width font and/or not applying syntax highlighting in terms of showing added and removed lines in different colors, that would make the diff more difficult to read.

But some mail clients can do that and it makes reading the diff much easier.

> I'm sure I will understand it better or even prefer it if I use it long enough, but then again I can instantly understand code reviews on GitHub/GitLab/Gerrit.

Essentially, code reviews in a mailing list are much like a discussion thread in Hacker News or Reddit where the thread structure is very similar. The only difference is that most mail clients only allow you to display one message at a time.

In my mail client, Thunderbird, you can see the overall thread structure of a patchset discussion. This is the root message for the thread which serves as the cover letter (which is the equivalent of the PR description) [2]. The first commit in the patch is displayed here [3] (note that I have a plugin that enables diff syntax highlighting). The email subject is the commit message title (with the [PATCH v2 1/3] tag prepended to it). The commit message itself is the beginning of the email, and the diff follows.

Unlike Github (and maybe Gitlab), the commit message and diffstat is treated at the same level as the diff itself. That means you can comment on it just like you would on the diff.

Here, you can see the Junio C Hamano's comments on the second commit in the patch set [4]. He's commenting on the diffstat line which shows 391 lines lines added to the builtin/submodule--helper.c file. Further down in the same message [5], he's commenting on the code inline, much like someone would quote a message here on HN and reply inline to multiple sections of it. It's not really that different compared to comments on a diff in Github or Gitlab other than the fact that it's a reply to an email message rather than a web page.

[1] https://i.imgur.com/QmqUWR8.png

[2] https://i.imgur.com/mILREtf.png

[3] https://i.imgur.com/gdoy5zs.png

[4] https://i.imgur.com/BcTdRRe.png

[5] https://i.imgur.com/cCpqsOL.png

[fireattack]

I will be honest, I don't even use email client :/

[u801e]

Now you're claiming that I'm making an argument that I never made.

To be clear, the original statement was whether there was a better review tool compared to Gerrit for those who review code and use that tool for that purpose. I responded by suggesting the patch review via mailing list method and asked a follow up question about how Gerrit handled related commits and explained how that case was handled by the mailing list method.

Then you, not the person I originally responded to, decided to interject and, while claiming not to be rude, claims I'm saying something entirely different than what I actually stated.

Personally, I found that very off putting and extremely rude on your part.

[AaronFriel]

I'm sorry I was rude to you, I didn't mean that and I apologize.

I didn't mean anything more than what I literally wrote, which is I don't think you're the target market for code review tools and your suggestion may not be a good fit for people looking for review tools.

[waheoo]

Gerrit handles related commits in a similar way I guess.

A Gerrit changeset is like a GitHub pr, it has many commits, and commits are usually rebased against master, this is crucial to track the review properly over time. (Its very easy to switch to earlier revisions of the same changeset and you never see external changes.)

Honestly, I don't think Gerrit is anything special, I think it simply has the right approach to development (rebased/ff-only) that enables easy review.

[u801e]

Based on the documentation I read, it looks like Gerrit handles this by grouping changes by topics [1]. I'm not sure whether that can be done automatically when pushing up changes that span multiple commits in a local branch.

If I create a branch with several commits where one commit adds a new method with associated unit tests, and a subsequent commit adds several calls to that new method in the code base (while updating any affected tests), then how would Gerrit handle the ordering of those commits. Even if they're in the same topic, I don't know if there's a way to ensure that the first commit is reachable from the subsequent commit.

[1] https://gerrit-review.googlesource.com/Documentation/intro-u...