views:

212

answers:

2

I am in the process of implementing a review process for my small team (3 members).

We are using git, and the model we want to use is integrator with blessed repository. Every developer has a public repo, and the integrator pulls the commits to include into the blessed repo.

I see several alternatives for including reviews in this new process:


1. Developers are left in their sandbox

  • Developer A develops a new feature in a branch called feature published in his pub, then asks for a review from reviewer B.
  • Reviewer B fecthes feature to his local remotes/A/feature, and review the changes, gives feedback to A, A make the changes.
  • Repeat until B accepts the state of the feature branch.
  • B marks the last commit (Reviewed-by tag, I'm not sure how that works yet).
  • B publishes the reviewed branch to his pub, and asks the integrator I to integrate the changes to the blessed repo.
  • everybody can pull the changes from the blessed repo.

pros:

  • integrator can refuse non-reviewed commits/branches.
  • the marked commit shows who reviewed the new feature.

cons:

  • quite a cumbersome process. Or maybe not?
  • the review comments themselves are not stored anywhere for future reference (more than per mail, maybe).

2. Developers add their new feature in a branch of the blessed repo

The blessed repo is not so blessed anymore, but its master branch is.

  • A develops a new feature in a branch, and pushes the branch to the blessed.
  • A asks for review from B.
  • B reviews the branch feature from the blessed, gives feedback.
  • A and B work back and forth until the feature branch of the blessed is accepted.
  • B marks the last commit (Reviewed-by).
  • Integrator I can merge the feature branch to the master.

pros:

  • a little simpler

cons:

  • everybody has write access to the blessed, which means everybody might potentially wreak havoc in there.
  • the many details of the back and forth changes during review may not have their place in the blessed?

3. Third party review tool

After browsing for a while I found reviewboard, which seems nice (if we can get through the install on our server).

pros:

  • the entire review is available for future reference

cons:

  • setting up the tool on the server side
  • developer A should be able to have the unreviewed feature branch in his pub. How can the reviewer B mark the commit in A's pub as reviewed since it is read-only? Ideally the fast that a commit was reviewed should show from within git, without need to start reviewboard.
  • if the reviewed feature in A's pub is not marked by B, how can the integrator know if it was reviewed? By opening the third party tool, but is that not also cumbersome?

Since I want A to have his changes in his pub, I guess I want to do post-commit reviews. The question is what should happen once the review has passed, the integrator should be able to pick the feature to the blessed and see if it was reviewed, preferably without firing up the tool.


I am looking for comments on these alternatives, and other suggestions for the way to go! I can already tell that I am not fond of option 2.

+1  A: 

Actually, option 2, if setup with a gitolite-managed ssh access, can be perfectly viable:

You can protect the master branch from any commit.

A few comments:

  • option1: "publishes the reviewed branch to his pub": I suppose he tracks it with a local branch
  • option2: "Integrator I can merge the feature branch to the master": actually, ye can rebase feature branch on top of master, in order to keep the history of feature branch complete and facilitate debug if the code goes wrong.
  • option3: it involves some administration tasks (for the setup and maintenance of the external tool), but it is the only solution that will allow you to actually persist the reviews.
VonC
opt 1: yes, tracking remote. Opt2: the point is to integrate the new feature to the master, to promote it to integrated. To keep the history, I'd merge with `--no-ff`, maybe. Opt 3: agreed!
Gauthier
+1  A: 

We are using github as a central repository for the developers to push to when something is ready to be shared. e work in a branch per feature/ticket/task so they are identified by the JIRA id. This keeps the scope nicely traceable.

Github has some pretty good support for code reviews which works very well in practice. Reviewer can attach notes to lines in the deltas or code and these are mailed to the concerned. The status of the feature is tracked through the Jira status.

In our case there is a CI server monitoring this server.

I have been thinking to pull from this server the completed and reviewed features to a "blessed" UAT repository to start the transfer to production in a controlled way.

This saves us from the hassle of maintaining a review server, we have persistent record of review comments, traceability of work scope to code deltas, it is continuously integrated.

The con is that the code is externally hosted which costs some money to keep it private, and there is the additional user management with private keys to give developers access. Getting over the firewall was solved with corkscrew.

We find the advantages largely outweigh the disadvantages and are infecting other teams in our organisation.

Peter Tillemans