[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: Adding reviewing to our development process



On Fri, 2008-10-24 at 12:38 -1000, David Cantrell wrote:
> What about a process like this:
> 
> * Create new review branches for the active development trees.  One for
>   rawhide, one for rhel4-branch, one for rhel5-branch.  Whatever we need,
>   really:
> 
>       master-review
>       rhel5-branch-review
>       rhel4-branch-review
> 
>   Or some other names (rawrawhide, for example).
> 
> * If you want code reviewed by someone else before it goes in to the mainline,
>   commit it to the review branch for the release you're working on.  A commit
>   mail will go out to everyone and we will also see code come in if we are
>   keeping local copies of the review branches (which we all should anyway).

>From a process point of view, doing "if you want ..." means "will rarely
happen".  So it probably should be a required step, although we can
certainly start with it just being required for a specific branch.

> * Establish review rules.  Such as, you can't review your own code.  Someone
>   else on the team reviews the patch on the review branch and if they like
>   it, cherry-pick it to the mainline and use the Signed-Off By feature of
>   git.  That way we get two people to blame if a patch goes bad.

Yeah, definitely have to have some things like this in place.

> By using review branches to stage code, we can all continue to work at our on
> pace and still get reviews from other people.  We can at least ensure that
> what gets built from rawhide is more stable...or at least looked at by two
> people.

My biggest concerns with doing it with review branches as opposed to
mail are
* The review branches could end up with intertangled commits.  Thus, if
something later is more "pressing", cherry picking it over could be
difficult
* Cherry-picking/pulling things over is going to make the commits list
more chaotic (every commit will be sent to the list twice)
* Reviews are less explicitly public

Jeremy


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]