Code review required for commits - Discuss.

Ricky Nance ricky.nance at weaubleau.k12.mo.us
Thu Oct 11 22:32:44 MDT 2012


Sorry if this is a dumb question, just trying to understand. What happens
after 3 days if the code hasn't been reviewed and signed off on, does the
patch die, or does it have to be resubmitted, or does someone else review
and push it? Like I said, I am just trying to make sense of how this works.

Ricky

On Thu, Oct 11, 2012 at 11:26 PM, Ira Cooper <ira at samba.org> wrote:

> On Thu, Oct 11, 2012 at 11:52 PM, Matthieu Patou <mat at matws.net> wrote:
> > On 10/11/2012 06:58 PM, simo wrote:
> >>
> >> On Thu, 2012-10-11 at 18:23 -0400, Scott Lovenberg wrote:
> >>>
> >>> On Thu, Oct 11, 2012 at 4:47 PM, Ira Cooper <ira at samba.org> wrote:
> >>>>
> >>>> On Thu, Oct 11, 2012 at 4:33 PM, Matthieu Patou <mat at samba.org>
> wrote:
> >>>>>
> >>>>> On 10/11/2012 12:48 PM, Ira Cooper wrote:
> >>>>>>
> >>>>>> Actual formal suggestion:
> >>>>>>
> >>>>>> No team member commits their own code.  All code will be "signed
> off"
> >>>>>> by two team members, as a team member you may sign off your own
> code.
> >>>>>> The "non-author" team member will be responsible for pushing the
> code.
> >>>>>>    If there are two they can agree among themselves. ;)
> >>>>>
> >>>>> To my french mind the wording seems a bit confusing, are you saying
> >>>>> that:
> >>>>> 1) every author could (should?) add their own sign-off
> >>>>> 2) in total 2 sign-off are need, which means at least 1 non-author
> >>>>> review
> >>>>> (but can be two if the author decide not to sign-off it's patch)
> >>>>
> >>>> That is correct.
> >>>>
> >>>> Time to time we all write things that need some more review, and we
> >>>> know it.  Not signing them off is a way to say "Hey, I want a second
> >>>> reviewer, because I know this is hairy."  I actually have some code
> >>>> sitting around that may trigger that, because of what it plays with
> >>>> :).
> >>>>
> >>>> Also it covers all "non-team" code, they all go through 2 sign-offs.
> >>>>
> >>>> -Ira
> >>>
> >>>
> >>> Ira, we discussed this a bit in the IRC channel today, but I'm
> >>> curious.  How does this work for a non-member that has code in Samba?
> >>> Can I maintain the parts of code that I've written without two team
> >>> members signing off?  It's really not a big deal, but we might as well
> >>> get these things ironed out while there is discussion going on.
> >>
> >> As far as I am concerned 1 signoff (author) and 1 ack should be
> >> sufficient, independently of who is the author.
> >> However it would be really desirable that who acks is the maintainer.
> >
> > Why not at the opposite encouraging review by someone not necessarily
> > familiar with area of the code ? so that we tend to encourage
> involvement in
> > parts where we usually don't work ?
> >
> > As for sensitive part people concerned about the patches (ie.
> maintainer(s))
> > could always pop-up on the list and tell that they want to do a review as
> > well and do it within 7 days.
>
> Matthieu,
>
> You got it 1/2 right, I agree people should do what they can to expand
> as they can.  But the whole "maintainers" system is the 100% anthesis
> of that.
>
> No kings, no popes.  If you are respected, people will defer to you
> naturally.  Order should come from the chaos naturally, not a file.
>
> "MAINTAINERS" should come out as a cultural convention.  People defer
> to those more knowledgable.
>
> As far as the time duration.  I stand by 3 days.  If you can't get to
> it in 3 days.. you aren't getting to it.
>
> -Ira
>



--


More information about the samba-technical mailing list