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

We use Gerrit. Gerrit is a review system derived from the Git technology.

From the Git user's point of view, Gerrit is installed as a git remote. When you develop some changes, you push them to the Gerrit remote, using a special branch naming convention. Gerrit dynamically takes this push and converts it to a review item, which is then subject to a web-UI-with-email-notifications-based review workflow.

A special "Change-ID:" item in the commit header identifies the change. This lets you push revised versions of the commit: if the Change-ID matches, Gerrit recognizes a push as being the new version of a previous push. These versions are organized in the review process under one item. You can see what the differences are between different versions and so on.

The review process involves multiple users in reviewer roles. They get notified via an e-mail that there is something for review. Then they can look at the changes, and approve or reject the commit. General comments can be added to the item and comments can be attached to individual lines of code, also, so you can easily say very specific things about specific pieces of code.

We have automatic reviewers who participate also. For instance, if a kernel change is proposed, then "checkpatch" is run on it automatically, and posts an approval or disapproval to the Gerrit item. The code also has to build for several targets and pass automatic smoke tests.

Commits are ultimately merged by users who have the power to do that. They take into account all the other votes and then cherry pick the change.

Gerrit is far from perfect, but it provides a decent workflow, and is a huge improvement over ad hoc development. For instance, the Change-ID can be a pain. It's assigned by Gerrit when you first push something, and then you must remember to add that same Change-ID to the new version of the commit. I wrote myself a script which generates a Change-ID; I stick it into the very first commit, so Gerrit doesn't have to make one up for me.

To use Gerrit, users have to acquire a somewhat better than average understanding of git; it's just a little bit more advanced than the "pull, commit and forget" use cases whereby you pretend that git is SVN or CVS. Things should be set up to prevent common mistakes; for instance, lock the repository from direct pushes, so only pushes to Gerrit are allowed. I would make a blank "Change-ID:" part of the commit message template, and give users a way to generate these things, to avoid the common mistake of pushing a new version of a commit without a Change-ID, causing Gerrit to generate a whole new review item.



All of the problems you mention are completely, 100% solved:

https://m.mediawiki.org/wiki/Gerrit/git-review

You're welcome ;)


The problem is that it doesn't just solve the couple of minor annoyances, but provides a whole new ball of wax.

I don't want to work with anything that automatically creates, switches or deletes branches.

I happily work with Gerrit using nothing but the regular integration branch: very simple.

If I have several independent changes, I just keep them in that same branch as if they were dependent. This does generate some nuisance rebases in Gerrit, but these are not worth the hassle of separating the changes to different branches. If I happen to have a large number of independent changes, what I can do to avoid overwhelming the review system is to not push all of them at once. I can do a "git rebase -i" to re-order the changes in the order in which I would like them approved (say by urgency of the issues to which they are attached). Then, piecemeal, I can push these out to Gerrit, say two or three at a time. When those are approved, push the next batch and so on. This keeps most of the "false dependency rebasing" local to me, not bothering the reviewers.

Gerrit requiring a somewhat better than casual familiarity with git is a good thing; by requiring it, it thereby promotes better familiarity, which makes developers more productive in the use of their version control system outside of the Gerrit context also.

The Change-ID handling is a very minor thing. When making a first commit, I just do ":r!change-id" in Vim, and it reads in the output of my change-id script: a brand new Change-ID: line with a shiny new change ID. My Git commit message template contains a blurb which reminds me to do this. We have a required bug number field also, so this just goes hand-in-hand with that.


I have created literally thousands of reviews with git-review, and I have never once used it to create, switch or delete a branch.

When I clone a new repo, I run "git review -s" (for setup), which installs the commit hook that ensures I never need to give even a passing thought to Change-IDs.

Then I run "git review" to push changes to review. I never have to think about where I'm pushing them because it's all configured in the .gitreview file. I generally also keep all my changes in a single branch (although I also use Stacked Git to cut down on unnecessary dependencies, and because I like it a lot better than "git rebase -i" for many purposes).

That's it. I've heard there are other flags. I've never used them. Nobody is forcing you. But by all means carry on generating Change-IDs manually if you like.


git review to submit, git review -d branch to review.. too bad the ux is so bad


I think you might be confused... (and if not then I certainly am). The -d flag means 'download', and the argument isn't a branch name, it's the review number from the Gerrit URL. You don't use it for reviewing code (that generally happens in the Gerrit web interface, or in gertty), you use it when you want to modify a patch submitted by somebody else. I'm not sure what a UX that lets you check out somebody else's patch without specifying which patch would look like.

FWIW I use Gerrit every day, and I don't recall ever once using the -d flag.


If this is all that's wrong with the UX, why not write two tiny one-line scripts called gerrit-submit and gerrit-review?




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

Search: