Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question
    
Are you doing unit test before commiting? –  Guy Coder Nov 28 '12 at 23:14
    
@GuyCoder: Mostly, we do. –  DeepSpace101 Nov 28 '12 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 '12 at 0:55

2 Answers 2

up vote 8 down vote accepted

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

share|improve this answer
    
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 '12 at 23:55
    
@Sid With a team of 3 I wouldn't let them all push to master. –  Andrew Finnell Nov 29 '12 at 0:09
    
Pull requests are also available in Rhodecode and Atlassian Stash –  dukeofgaming Nov 29 '12 at 0:33
    
@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 '12 at 0:53

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?

share|improve this answer
    
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 '12 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 '12 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 '12 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 '12 at 0:39

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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