On Thu, 2008-10-23 at 09:51 -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.

One thing to consider is moving the commit spam to this list so that
it's more visible...

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

I think that the easiest "emergency escape hatch", although it's one
that is kind of painful and sucky, is patches in the src.rpm. It makes
it more clear that "this is not finalized".

Jeremy

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

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

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