[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Code Reviewing

From: David Weintraub <qazwart_at_gmail.com>
Date: Wed, 28 Jan 2009 12:58:45 -0500

I agree with you.

Simply reviewing code on our trunk, and making changes as necessary
would be my preferred method. To me, a code review should be mainly
catching minor issues (Hey, you should really check for a null pointer
exception here) and not major restructuring and corrections. That
depends upon assuming your developers are competent professionals who
know what they're doing and will ask for advice before doing some
major overhauling of the code or doing something non-standard.

Apparently, our project leads aren't too sure whether this assumption
of competence is true of all our developers -- especially the ones
where the code has been outsourced.

Promotional branching merely pushes the issue down the line. Same with
shelves. What happens is that you end up having to merge your code
into the production branch after others have made their changes. What
do you do at that point? Another code review? (Unfortunately, that's
the solution at many places. I've seen one developer stuck for weeks
in code review after coder review because after the initial code
review, he was forced to merge in other people's changes into his
"approved" code. Thus, another code review. Then again, and again. The
irony is that his code always passed the reviews with flying colors.
But, policy is policy. No checking in non-reviewed code into the
production branch).

I'll take a look at TeamCity to see if this can handle our issues.

On Wed, Jan 28, 2009 at 10:43 AM, Bob Archer <Bob.Archer_at_amsi.com> wrote:
>> How do you handle code review in Subversion? TeamFoundation has
>> something called a "shelf" that seems to be able to handle something
>> like this. Is there a similar concept in Subversion?
> Basically doing a "shelf" in TFS is nothing more than creating a private
> branch, switching to it, and then committing to that. TFS does all this
> for you in the background though.
> You could probably create a script or batch that does this for you. Once
> the branch ("shelf") has been reviewed it can be merged into trunk. You
> could create a batch to also do that too.
> I had seen talk somewhere, perhaps on the TortoiseSVN list to create a
> "shelf" command that would do this stuff for you in the background.
> Another option would be to integrate git into your work flow. Git
> basically stores private repositories that you can easily branch and
> people can push/pull stuff from one person's repo to another's. Then use
> git-svn to finally push this stuff to your svn repository.
> What we do, since we are a pretty small shop is just commit the code to
> trunk. We have a task in our SCM system for a code review. The dev just
> specifics the revision they want reviewed. That is picked up. If all is
> ok then there is no merge or anything needed. If there is a problem
> changes are just made right to trunk.
> BOb

David Weintraub
To unsubscribe from this discussion, e-mail: [users-unsubscribe_at_subversion.tigris.org].
Received on 2009-01-28 19:00:27 CET

This is an archived mail posted to the Subversion Users mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.