Re: SVN with code review workflow
From: BRM <bm_witness_at_yahoo.com>
Date: Tue, 30 Jun 2009 06:13:05 -0700 (PDT)
Reading over some of this I think there is a big point that needs to be made - what is it that the review process cares about? and what is it that a version control system cares about?
Your review process cares about the change that is being made to the product.
So think about it this way:
You have your product in the repository (say http://svn/product_name, so use another example) with its own trunk (e.g. http://svn/product_name/trunk).
The key to remember is that the developer makes many changes in the process of making that change that will be reviewed. They might create add 200 lines of code, modify 400 lines of code, and then delete 300 lines of code. The question comes - how, then, do you track it? Your review process doesn't really care that 200 lines were added, 400 lines were modified, and 300 lines removed; it really only cares about the net changes - 100 lines of code removed and 400 lines modified. However, the developer may very well care about how they went about those changes, and adding version control will give them the ability to manage those changes. Further, having a single version control system will allow the company to backup their work between the changes.
Okay - time for an example. One job I worked at did not have any version control in place. (No reviews either.) The original developer would make his changes to the code base and when he was satisfied copy the directory as a marker. Management didn't care that there was no version control system either. (sadly) So when I started, they wanted me to do the same. After a little work, I installed TSVN on my system, and checked in everything to a local repository on my laptop, and then exported to make the copies they wanted. This gave me the ability to version my changes - and track what I did - in an easy to use manner with full version control, but outside of management. If anything happened to my laptop, then all changes were lost - the whole history was lost. I later convinced them to give me a system to host SVN on, and converted my local repository into a hosted repository where others could gain access and we could do daily and monthly backups.
Fact is, your developers that care about version control are going to have a version control system installed on their own development systems - whether it is SVN, CVS, git, or whatever else they can get their hands on. As an organization, you will not be able to capture those changes in the history, nor make backups. Developers do care about all changes as they'll be able to undo things in their working copy a lot easier with version control than without when they find that a change did not work. Version control - any version control - makes this far easier, and I'd probably argue SVN makes it among the easiest. (There's probably a good sized group that would argue that git makes it easier yet - I don't know - haven't used it; but that's a different topic altogether.)
So the organization is faced with the question: What is it that we care about? Capturing the history of changes to the product and every step of the way? Or capturing only the major changes? Backuping up the developers work and aiding them in the development process? Or making the development process a non-trivial task?
A smart manager will recognize that version control is an aid to help the developer AND the organization, and the permissions and layout of the repository aid the process the organization wants to use.
Further, permissions would also allow you to assign a group of developers to work on a single task - and they could branch/integrate between themselves as well if the repository supported it.
So in the end, version control gives you tracking of all changes, you can still have your review process, and you can get a more collaborative development environment that is also more developer friendly in the process with a code-base that has its full history and can be backed up as desired by the organization.
First of all, thanks to everyone for the helpful replies.
You're absolutely right. I believe this is a social problem in our organization. Those in charge treat everything with a militaristic rigidity and at this point are absolutely unwilling to give up on the idea that everything must be code reviewed before being (currently manually) merged with the main development codebase. Adding to the pain for developers is a 4 hour turnaround time and a ridiculously strict set of standards (i.e. if you have an extra space in your code, it will get kicked back from review). But the point is, this code review policy is the excuse for not having Subversion in place. At this point I'm just trying to come up with an irrefutable setup using SVN that will be more productive than what we have currently.
I see, so you are suggesting that you basically get with the reviewer one-on-one before the commit is made, perhaps at the desk of the developer, to look at the changes, and then after everything is approved, make the commit. Unfortunately, at this point, I think the social aspect is at a point where those in charge will not "allow" the code-reviewers to meet at the developers desk to look over the code, and instead the reviewer must review the code independently at his own workstation, then return the results to the developer. At which point, changes are made, and resubmitted, and the 4 hour clock starts ticking again.
>>If you really want some enforcement, a pre-commit hook could reject
This is a great idea and one I will bring up when I go to fight the good fight. Sadly, at this time, not even typos can sneak by the 4 hour review turnaround.
Understood. But compared to what we have now it sounds like a weight of developers' shoulders already. Russ suggested earlier different branches based on features / tickets, etc., but with the amount of projects we have running through here, I think they would argue that this would get far too cumbersome very quickly (not that I agree, just looking to prepare myself here). Also, others have suggested setting up a branch before a launch and taking care of everything there before commit. The thing is, here, we do sort of a piecemeal deployment, each small project at a time, so bundling everything together into a new branch for "Version 1.6" or something would be shot down I'm sure. I'm thinking continuous integration is the solution to this, but first thing first and I'm just trying to get Subversion in place as priority number one.
Also, Les made a comment: "Just because someone commits a change to the trunk doesn't mean you have to use it in production." What you say is true, of course, but is there a good way to keep track of what should and shouldn't be part of a deployment when there are production ready and non production changes being committed to the same trunk? Or are you just suggesting that another committ can follow right behind to clean it up?
What I'm getting at is, right now we manually (with an inhouse tool) keep track of what should and shouldn't be deployed by means of creating a "depoyment script" which says, ok, methodA, methodB, methodC, and viewA, viewB, viewC, and imageA, should be part of this deployment, and nothing else. I think the other argument I will get is "Using Subversion, with all the projects we have going on simultaneously, how are we going to keep track of what items should be part of each deployment?"
> I have also heard of tools such as Codestriker
I've used Review Board
I'm not sure I follow here. They mail and later apply the patch? What do you mean by this?
Another great idea, thank you.
Very true. Thanks to everyone for their thoughts, you guys have given me some good fuel.
To unsubscribe from this discussion, e-mail: [users-unsubscribe_at_subversion.tigris.org].
This is an archived mail posted to the Subversion Users mailing list.