Re: [stellation-res] Code review process

["Mark C. Chu-Carroll" <mcc@xxxxxxxxxxxxxx>]

I'm against this. My reasoning, roughly, is that this won't eliminate
any problems; it will merely make them harder to detect.

The problems with patches come about when the workspaces of the patch
creator, and the workspace of the patch verifier are not in synch. Doing
the patch creation manually by  packaging changes files won't eliminate
that error.

If you're using an absolutely clean workspace as a starting point, and
making a single change in that clean workspace, and you generate patches
from the project itself, you're essentially doing exactly what's being
described below - only you're creating the patches by hand, by figuring
out what files you changed.

If you make a mistake doing this using patches, the patches will reveal
the mistake. If you make a mistake doing his manually, it will be easy
to miss. You lose the systems ability to help you view what has changed;
and you lose the systems ability to tell you when something is wrong.

To be honest, I'm frustrated with the code review process as a whole. I
don't think it's working out.

In theory, it's a great idea. In practice, there just aren't enough
people involved, so patches don't get checked quickly enough, and we
constantly run into problems because we're not in sync, because we went
ahead and started our next task while waiting for the last patch to be
verified. 

In favor of the patch process, I've liked the way it helps me keep track
of what's going on: I can see things happening in bugzilla, and it's
easy for me to look at an incoming change in the patch viewer and see
exactly what was changed. That's been very helpful to me. But overall,
I think its negatives are outweighing its positives.

	-Mark



On Wed, 2003-03-26 at 05:10, Jonathan Gossage wrote:
> There seem to be some problems with the current patch based code review
> process which center on the patches themselves. I would like to propose an
> alternate possibility which would have the side effect of taking us closer
> to the process that we will likely adopt once we start self-hosting.
> 
> The proposal relies on the use of multiple workspaces and on the use of file
> deltas instead of patch files.
> 
> It could work like this.
> 
> 1. Create a base workspace. This workspace would be kept in sync with the
> repository but would never have local modifications made to it. This
> workspace would be used for quickly creating clones in which actual work
> would be done.
> 
> 2. Create a separate workspace for each work element such as a bugzilla fix
> or enhancement.
> 
> 2, When ready to submit a patch, create a zip file that contains copies of
> all files changed while developing the patch.
> 
> 3. The reviewer creates a new workspace when the patch is received and
> unzips the patch into the workspace. The patch can then be reviewed and
> tested.
> 
> 4. Once a patch has been verified it can be checked into CVS by either the
> reviewer or the originator of the patch.
> 
> This process should get around the problems of creating patch files that
> cannot be applied.
> 
> Thoughts
> 
> 
> Regards
> 
> Jonathan
> 
> _______________________________________________
> stellation-res mailing list
> stellation-res@xxxxxxxxxxxxxxx
> http://dev.eclipse.org/mailman/listinfo/stellation-res
-- 
Mark Craig Chu-Carroll,  IBM T.J. Watson Research Center  
*** The Stellation project: Advanced SCM for Collaboration
***		http://www.eclipse.org/stellation
*** Work: mcc@xxxxxxxxxxxxxx/Home: markcc@xxxxxxxxxxx