12

What is the best process for code review when using GIT? We have an external GIT provider (Unfuddle) and have caps on resource usage - so we can't have dedicated remote repositories for every dev.

Current process:

  • We have a GIT server with a master branch to which everyone commits
  • Devs work off the local master mirror or a local feature branch
  • Devs push to server's master branch
  • Devs request code review on last commit

Problem:

  • Any bug in code review are already in master by the time it's caught.
  • Worse, usually someone has burnt a few hours trying to figure out what happened...

So, we would like

  • To do code review BEFORE delivery into the 'master'.
  • Have a process that works with a global team (no over the shoulder reviews!)
  • something that doesn't require an individual dev to be at his desk/machine to be powered up so someone else can remote in (remove human dependency, devs go home at different timezones)

We use TortoiseGIT for a visual representation of a list of files changed, diff'ing files etc. Some of us drop into a GIT shell when the GUI isn't enough, but ideally we'd like the workflow to be simple and GUI based (I want the tool to lift any burden, not my devs).

Improve this question
3
  • Are you doing unit test before commiting?
    – Guy Coder
    Nov 28, 2012 at 23:14
  • @GuyCoder: Mostly, we do.
    – DeepSpace101
    Nov 28, 2012 at 23:45
  • If you host does not provide code review capability, get a better host. Have a look at Gerrit, and see if you can find a host that provides it.
    – mattnz
    Nov 29, 2012 at 0:55

2 Answers 2

Reset to default
16

A simple but effective model is the GitHub pull request model, where contributors file "please merge in my code" requests. A maintainer reviews the changesets and decides if they need more work or if they are suitable for merging. He then may merge into the master branch. Committers are generally not allowed to push directly to the master branch (this can be customized to your tastes, we allow "minor" commits to go directly in).

Improve this answer
4
  • We have a tight team of 7 professional devs (vs anonymous contributors) globally, so its safe to let each one directly push to our remote master. Although it's a link+intro vs a standalone answers that I prefer, in this case it makes sense. Great writeup at link, thanks!
    – DeepSpace101
    Nov 28, 2012 at 23:55
  • @Sid With a team of 3 I wouldn't let them all push to master.
    – Andrew T Finnell
    Nov 29, 2012 at 0:09
  • Pull requests are also available in Rhodecode and Atlassian Stash
    – dukeofgaming
    Nov 29, 2012 at 0:33
  • 2
    @Andrew: Why not? There are many problems that can be generated by funneling the entire teams work though a choke point. These can all be mitigated, but a single point command and control structure is more suitable for some situations than others.
    – mattnz
    Nov 29, 2012 at 0:53
2

Git is a distributed version control system: don't have only one repo with one branch!

You can set up multiple repositories - one for each developer - and another which is the master repo. When one of their branches is ready to be merged, the developer requests a merge and their changes are pulled from their branch/repo into the master.

Before that merge actually happens, the reviewer can pull the changes into their environment, and review them before pushing them to master.

Added advantages are that this way a developer can have as many branches as they want, named whatever say they want, without interfering with each other or even having to see each other's dirty laundry that much.

Also, learn the lingo: by "Devs commit to server's master branch" do you mean that they push their changes to master?

Improve this answer
4
  • yeah, they push their work. We can't have unique remote repositories on the GIT server because we pay a GIT hoster and they charge per repository. Did you mean have multiple remote branches per developer? And when you say the reviewer can pull the changes into their environment what exact GIT commands (or TortoiseGIT flow) do you mean?
    – DeepSpace101
    Nov 28, 2012 at 23:49
  • No, I mean have multiple repositories; one per developer, and on that repository they can have as many branches as they want. As for pull, I don't know what the command would be in TortoiseGIT - but the command is git pull. It's the opposite of a push - you pull changes from the remote repository to update your environment with work other devs might have done.
    – Richard JP Le Guen
    Nov 28, 2012 at 23:53
  • I know git pull :) ... I was asking for the full syntax to inspect the repo/branch/tag system for push/pulls you were referring. Right now we do git pull anyways, it's just off remote:master - which causes the issues. Anyways, Steven's links was great. Thanks
    – DeepSpace101
    Nov 28, 2012 at 23:58
  • To all practical purposes in this case a branch in the hosted repo is just the same as another repo.
    – mattnz
    Nov 29, 2012 at 0:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.