Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Wikimedia is moving to Gitlab

mediawiki.org

Post reply on HN
Print view View original

351–360 of 408 posts

Re: Wikimedia is moving to Gitlab

#351

Post by JoshTriplett »

bawolff wrote: post #324
atombender wrote: post #318

Earlier quoted context omitted.

Gerrit is the one where each "pull request" has to be a single commit, right? I'm not particularly happy about GitHub, but I think it's less about GH and more about workflow and source code evolution, and I'm not sure if Gerrit solves anything here. First, PRs being heavyweight encourages large branches instead of smaller incremental changes. Secondly, a large change (such as a big new feature) ends up living in a br…

> Gerrit is the one where each "pull request" has to be a single commit, right? Yes, but you can have changesets depend on each other, so its not that big a deal. (But you ca end up in rebase hell if you do that). You also get a version history of all the different versions of your commit. Ancedotally, during my use of gerrit, i never really wished to have multiple commits on a single changeset. > I'd prefer to merge…

> Ancedotally, during my use of gerrit, i never really wished to have multiple commits on a single changeset.

People's tools shape their workflows; projects that use Gerrit tend to do more squashing of commits, because there's much more per-commit overhead. When I encountered Gerrit, I found it really frustrating to work with for this exact reason. Other aspects of it were great, but if you're used to "one logical change per commit" and end up with a dozen commits in a PR, that can be painful with Gerrit.

Re: Wikimedia is moving to Gitlab

#352

Post by jimmydorry »

Eldt wrote: post #316
NorwegianDude wrote: post #313

Earlier quoted context omitted.

Cool project, but I do find it quite ironic that it's named friendly captcha when it's not a captcha.

How would you define "CAPTCHA"?

CAPTCHA: a computer program or system intended to distinguish human from machine input, typically as a way of thwarting spam and automated extraction of data from websites

I would say this Oxford Languages dictionary definition is close enough.

Re: Wikimedia is moving to Gitlab

#353

Post by perryizgr8 »

Eldt wrote: post #316
NorwegianDude wrote: post #313

Earlier quoted context omitted.

Cool project, but I do find it quite ironic that it's named friendly captcha when it's not a captcha.

How would you define "CAPTCHA"?

The original expansion was "Completely Automated Public Turing test to tell Computers and Humans Apart".

Re: Wikimedia is moving to Gitlab

#354

Post by u801e »

fireattack wrote: post #348
h0l0cube wrote: post #314

Earlier quoted context omitted.

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?

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.

> 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

Re: Wikimedia is moving to Gitlab

#355

Post by fireattack »

u801e wrote: post #354
fireattack wrote: post #348

Earlier quoted context omitted.

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.

> 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 clie…

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

Re: Wikimedia is moving to Gitlab

#356

Post by u801e »

fireattack wrote: post #355
u801e wrote: post #354

Earlier quoted context omitted.

> 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 clie…

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

True, I suppose most people use Gmail or one of the other major email providers through a webmail interface. I haven't been able to get Gmail or Hotmail to display threaded messages the way they're displayed in Thunderbird and they tend to display messages using a variable width font.

In that context, reviewing code would be difficult, if not impossible, to do via email.

Re: Wikimedia is moving to Gitlab

#357

Post by h0l0cube »

u801e wrote: post #356
fireattack wrote: post #355

Earlier quoted context omitted.

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

True, I suppose most people use Gmail or one of the other major email providers through a webmail interface. I haven't been able to get Gmail or Hotmail to display threaded messages the way they're displayed in Thunderbird and they tend to display messages using a variable width font. In that context, reviewing code would be difficult, if not impossible, to do via email.

It is one advantage to mailpatch though. A lack of vendor lock-in means you can view the patch using whatever application you want. And there's room for a better mailpatch viewer, if anyone could be bothered to make one.

I'm guessing one disadvantage is the method of diff is hard-coded into the patch. It would be good to switch to word-diff, or ignore whitespace, but I'd imagine these could be applied as transformations on the generic format.

Re: Wikimedia is moving to Gitlab

#358

Post by u801e »

h0l0cube wrote: post #357
u801e wrote: post #356

Earlier quoted context omitted.

True, I suppose most people use Gmail or one of the other major email providers through a webmail interface. I haven't been able to get Gmail or Hotmail to display threaded messages the way they're displayed in Thunderbird and they tend to display messages using a variable width font. In that context, reviewing code would be difficult, if not impossible, to do via email.

It is one advantage to mailpatch though. A lack of vendor lock-in means you can view the patch using whatever application you want. And there's room for a better mailpatch viewer, if anyone could be bothered to make one. I'm guessing one disadvantage is the method of diff is hard-coded into the patch. It would be good to switch to word-diff, or ignore whitespace, but I'd imagine these could be applied as transformati…

> I'm guessing one disadvantage is the method of diff is hard-coded into the patch. It would be good to switch to word-diff, or ignore whitespace, but I'd imagine these could be applied as transformations on the generic format.

The plugin I use in Thunderbird can switch between unified, context, and side-by-side diff views based on the same email. Adding the transformations you mentioned could be done.

But one limitation that email has over other review tools is the lack of ability to expand the view of the context within the client. The only way I could think of is to have git format-patch generate the diff with the entire context included and then have the client limit the display of that context. But that not have a reasonable fallback for those using clients that aren't capable or configured to do that.

Re: Wikimedia is moving to Gitlab

#359

Post by u801e »

waheoo wrote: post #335
u801e wrote: post #310

Earlier quoted context omitted.

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…

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…

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...

Post reply on HN