Re: Adding reviewing to our development process

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

 



On Thu, Oct 23, 2008 at 09:51:46AM -0400, Chris Lumens wrote:
> > As I've been slowly getting into anaconda development I've become 
> > increasingly amazed with the number of regressions we have caused by 
> > typos and other small predictable errors in day to day development work.
> >
> > So I've been thinking that we really need to change our processes to 
> > include a review phase for all patches except the most trivial ones (the 
> > ones fixing all the typos).
> 
> Two things:
> 
> (1) We already have the commit mails, which we theoretically could all
> be reading and finding problems in.  Yet, I don't see a lot of followup
> comments in IRC regarding typos.  Jeremy has some from time to time, and
> I do too but very rarely.  I wonder how effective it's really going to
> be.
> 
> (2) If we go down this road, we need to make sure it's not mandatory.
> That is, I still need to be able to commit emergency things to do a
> build right now without having to wait for other people to comment.
> That's just the nature of anaconda development from time to time.

We do have the commit mails and most of us read them, but sometimes not.  It's
not really guaranteed that someone will review the code.  Plus, it could be
too late once someone reads it since a build could have already gone through.

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

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

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.

Using review branches also keeps the door open to emergency fixes that need to
go and we don't really have to change our processes for that.

Thoughts?

-- 
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI

Attachment: pgpLD12B9ztDR.pgp
Description: PGP signature

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux