Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

This is sad. In my experience, Gerrit is a much better code review system than Gitlab merge requests. But it is different from what people are used to.


Probably because you got used to Critique at Google ;)

I agree though. I think the most important thing in a code review system is inline comments in the diff itself, and that’s something you get from Gerrit, Phabricator (Differential), etc. It encourages people to discuss the particulars of of a diff. Merge approval can be made contingent on resolving minor issues within a diff. Diffs are also approved on a per-diff basis, and it’s less typical to merge a stack of diffs.

I think the pull request / merge request makes sense with the “trusted lieutenants” development model that the Linux kernel uses, but for other projects you would be more likely to want a work flow where someone submits a single commit/diff and then someone approves it (after comments).

When I review PRs on services like GitHub I very often think, “This should be several different reviews” and the discussion thread in a PR is often not a high-quality discussion. I don’t use GitLab as much but my experience is that it has the same problems. What I would love is to review a stack of commits and approve / make comments on the commits individually.

(For those reading: Mondrian -> Rietveld -> Gettit, and also Mondrian -> Critique. Mondrian and Critique are internal tools at Google. Phabricator originated at Facebook which has a lot of ex-Google engineers on staff.)


I don't use Github much, but Gitlab allows for multiple threads in a merge request. These threads may reference diffs/commits, but can also be directed at the merge request in general. Each thread has to be explicitly resolved before merging.


I think the pull request model still makes sense. Of course if you stick to small changes it tends towards the patch model. However thete still some cases where two or three commits at once make sense. Even rarer there are cases where marging a bunch of changes into a "staging" branch before merging to master makes sense. I think this added flexibility is valuable, of course keeping the "patch style" single-commjt review great should probably be the priority.


Do you think that gitlab will ever add inline diff comments? I don't know if it would even be feasible to add to gitlab


More online than comments on the changes in the Mr? Like: https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/2...

(there's some old discussion on there, random example)


I guess you can get the hang of Gerrit, but i tried it out a couple of times (i occasionally do work on projects in the Wikimedia git repo) and is pretty evident that the interface is written by developers with little knowledge of user experience. Moving stuff to Gitlab will probably increase the number of volunteer contributions, at least i'm more interested in contributing more now.


I really don't mind the Gerrit UX - it seems to be optimized for daily use by programmers, not for onboarding speed. That's a tradeoff I'm very much okay with.


Gerrit is so off-putting to new users that many never get over the learning curve. At Wikimedia we want to be welcoming to new contributors. We also have to consider the on-boarding experience of new staff members, as well as the productivity of staff and long-time contributors. Gerrit satisfies some people who have used it for a long time but it is almost universally disliked by newcomers. When your users are volunteers, you can't force them to use Gerrit until they get used it it. If the experience is bad enough then they will choose to spend their time on something else instead.


I'm personally somewhat doubtful that that is really the aspect of the new dev experience at wikimedia that is actually turning off newbies.


Curious, did you think of moving also the repos and reviews to Phabricator, as I recall WMF using that for task management?


You're okay with the tool being difficult to use because you already know how to use it.


I'm okay with it being slightly more difficult to get started with, in return for higher productivity in the long run, yes. Right now, after spending a similar amount of time both with GitHub PRs (and Gitlab MRs) and Gerrit, I still find Gerrit much easier and faster to use.


I'm generally okay with that tradeoff too, but we tried it early on at our company and the juice was not worth the squeeze, at least in our case.

Designers and even many developers found it essentially impossible to use and the developers who were reasonably comfortable with it spent way too much time assisting others in attempting to use it.

(fwiw I found myself somewhere in the middle - I like the model and understood the ideas but also found it annoying to work with in practice)


Same. Cool idea in concept. Not something I have enough time to be interested in using heavily.


I'm okay with Vim being slightly harder to learn to use than VS Code. A tougher learning curve in exchange for more powerful tools can be a good tradeoff.


I think this is a good comparison, and exactly shows the problem: Vim is only used by a minority of developers, the majority use some kind of graphical editor (like VS Code). That doesn't mean learning Vim isn't a good tradeoff, it's just not a good tradeoff for the majority of people.


I already know how to use it and I hate how proprietary it is, I have so many other problems on my plate than customizing my Git to work a certain way. I like using the off the shelf tools that work nicely with the normal git workflows using temporary branches for MRs. And a nice UI that anyone can use with minimal effort or training.


Difficult to learn != difficult to use.


I agree with you - for tools I use on a daily basis.

However, since my interests vary considerably, and therefore I dabble with lots of different tools, the difficult-to-learn tools never get enough traction in my limited human memory to get me to the easy-to-use stage.

If a community doesn't want to engage occasional users, it's probably fine (maybe even desirable) to have a higher barrier to entry to make daily use really fast.

If a community benefits meaningfully from occasional users, a high learning barrier may not be a good thing.


"Because it's hard" is a bad reason to shy away from something.


No it's not, you have limited time and devoting X hours to gain back X/10 hours worth of productivity gain in the future is a bad investment. Don't do something hard for the sake of doing it unless the gains out weigh the cost.

https://xkcd.com/1205/


Software should be written for users, not non-users. You'd think this would be self-evident and yet here we are.


How do you then convert a non-user into a user with the least friction?


Force. I for one never looked at Gerrit and thought "I should push this at my employer". I'll probably never use it unless I'm forced to.


Gerrit ux is full of bugs that get in the way of daily use (maybe improved over time). Things like overriding ctrl-f in the browser but then having the overriden search bar not work or not being able to effectively type inline comments on mobile.

I dont really think the choices that they did make that work, are really any better in optimized daily use than intuitive choices would have been.

(Yes probably much of this is fixed)


The git-review [1] tool makes it trivial to interface with Gerrit (it's likely packaged for your distro, see [2]). I've found many people struggling with Gerrit don't know about it and it has made their life significantly easier. It handles all the magic of pushing to refs so that you never need to know about it. You drop a .gitreview in your project and then your work-flow is literally

    $ git checkout -b my-feature-branch
    $ emacs ... <edit edit edit>
    $ git add -i ...
    $ git commit
    $ git review
     read reviews, edit
    $ git add -i ...
    $ git commit --amend
    $ git review # push new change revisions
You can download an upstream change to use locally with "git review -d 123456"

[1] https://docs.openstack.org/infra/git-review/ [2] https://www.mediawiki.org/wiki/Gerrit/git-review


Honestly, i find git-review much more annoying than just memorizing `git push origin HEAD:refs/for/master` (if that's too much easy to create a normal alias) and the gerrit web interface gives you the command to download a specific changeset. Git-review tends to break in unclear ways and sometimes do things other than what i expect it to.


> Moving stuff to Gitlab will probably increase the number of volunteer contributions, at least i'm more interested in contributing more now.

Is there a project out there that originally used something like Gerrit, Phabricator, Reviewboard, or a mailing list that moved to Gitlab or Github where the number of contributions increased after the change?


What advantages do you see in Gerrit? Do they require a lot of experience in order to be realized?


Clear mapping of change request == commit, allowing for easy building of multiple in flight change requests, rearranging them by using git rebase, updating with push to refs/for/master. Easy diffing between states of the CR (patch sets), so you can see what changes since the last round of comments, even if it spanned multiple updates. This is the feature I miss the most from other code review systems - be able to easily work on another commit that bases on one that I just sent out for review (even starting review on the new while the parent still hasn't finished being reviewed!), and rebasing my current change as the parent change gets reviewed/updated.

Possibility to send out a PR for review using just a push (change message in commit, push to refs/for/master%r=foo).

Snappy and compact code review experience (no space wasted for whitespace, avatars, pretty buttons). Full coverage with keyboard shortcuts.

Powerful review rule system based in Prolog, allowing for things like code owners, experimental subdirectories without the need for review, etc.


> Clear mapping of change request == commit,

was forced to use Gerrit by a client. I could never get the hang of this, I like to do frequent commits on short lived branches and using vanilla git. I never wanted any more features other than a nice UI to encourage people to review.


The nice UI for review works well exactly because it limits the functionality available to users and enforces a particular commit model. If you don't do that, you get the code review mess that are Github/GitLab PRs/MRs - difficult to tell apart how commits relate to the change and how it progresses through review, because the entire branch history is free-form.


It's possible to allow users to have multiple commits but still show review between the submitted "tip" commits. If you want you don't even need to show the other commits in the UI.


One thing one has to get away from that one change request == one issue. Multiple small commits as separate change requests for one story are fine as long as they work standalone (which is generally a good idea, e.g. to enable bisecting)


I much prefer the model of force-pushing your development branch to create change sets. It lets you more easily see how development evolves in response to feedback. And the final state of the branch which gets merged leaves all the in-progress work that no one cares about behind only in Gerrit.

With Github/lab's model, if you force push your PR, you lose the ability to view its previous state and diff against that. Alternately, if you just keep adding commits, then the final branch that gets merged (unless you squash) has all the in-progress work which pollutes the repo's history.

Gerrit also has a finer grained permission model, but I don't care as much about that.

Gerrit definitely expects the user to understand how git works conceptually a bit more than Github/lab.


> With Github/lab's model, if you force push your PR, you lose the ability to view its previous state and diff against that.

That's not quite true. Gitlab lets you compare any two "versions" of the force pushed branch.


Thanks. I haven’t used gitlab. I assumed it worked the same as GitHub. That’s good to know.


Yup, GitLab works as expected here. It's always surprised me how quickly the old commit is garbage-collrcted when you force-push a branch on GitHub. It causes weird errors in CI runs and breaks viewing the old commits.

Seriously I'll pay for those couple of KiB of space, just keep it around. (at least until the PR is closed)


Gitlab DOES let you compare different versions of changesets in a merge request: https://docs.gitlab.com/ee/user/project/merge_requests/versi...


> With Github/lab's model, if you force push your PR, you lose the ability to view its previous state and diff against that.

I'm not sure about Gitlab, but Github has recently added a feature where you can view the diff between the old branch head and new one. But, as far as I'm aware, there's no way to check out the previous branch head from the repo due to a lack of a remote branch pointing to it.

At least git itself provides a range-diff command that allows you do see a diff between the commits between two versions of a given branch.


But you don't force push with Gerrit?

Just update your change set, then push to refs/for/<branch_name> again.


It's been a while since I used it, but I guess the more important point is that it's a rebase-based workflow.

My memory was you had to force push your working branch to refs/for. Thank you for the correction.

I've actually setup and run instances of it as two companies, but as I say, it's been a while.


Yes, I very much agree: the rebase-based workflow is what makes Gerrit superior to all other systems I've tried (of which I find Github and Bitbucket particularly loathsome).

I felt I needed to correct you because with Gerrit you reserve the concept of force pushing for exceptional cases, which I think is the correct mental model. Force pushing should not be done frivolously.


It's effectively similar to a force-push.


I would say it's quite different, since you don't overwrite anything. Old patch sets are still available should you wish to roll back/reference either a particular commit, or a whole chain of commits.


It is a force push under the hood. You’re removing an old commit and creating a new one.


It's not a force push under the hood. Under the hood a new branch is created (first patch set is /refs/changes/nnnn/0, second is /refs/changes/nnnn/1, etc.). From the end user perspective the result is quite plausibly similar to force push.


The branch model must have changed. I used to push directly to a branch that was the change number (not the revision number) and of course had to force because it was the same.

So what happens when you push to changes/nnnn/12 when revision 11 hasn’t been created?


At least the way our installation works we don't push to changes/nnnn/12, rather changes are pushed to refs/for/{branch} (often refs/for/master). This endpoint isn't an actual branch. It triggers gerrit to look up the nnnn for the Change-Id in the commit message and create the appropriate branch for the next patch set.


It's depressing how git tooling has to squish out all the flexibility that makes git attractive in the name of being unsurprising to someone who doesn't understand git.

I was excited to finally bring in git as we start ramping down on a legacy project onto something new. Then I started thinking about the developers that have never touched git and I need to support. I looked at the tools available and what workflows they dictate. Then there's the drive to do something similar to the rest of the company, autonomy only goes so far without a good reason.

Fuck me. I'm going to pick GitLab and hate it.


I had to use Gerrit in a previous job, and _hated_ it. The UX is abysmal. Some folks loved it though, especially engineers working mostly on the backend. People with more of a frontend focus couldn't get past the awful user experience.


Can confirm, it's extremely off-putting to say the least. Having previously used Github, Gitlab, Bitbucket I find Gerrit very unusual. I have to Google on how to do basic stuff.


Why is it sad? They've identified a better experience for their target audience, new community devs, just that not perhaps for you haha.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: