Code review required for commits - Discuss.

Jeremy Allison jra at samba.org
Fri Oct 12 11:22:49 MDT 2012


On Fri, Oct 12, 2012 at 01:41:01AM -0700, Matthieu Patou wrote:
> On 10/12/2012 01:20 AM, Volker Lendecke wrote:
> >On Fri, Oct 12, 2012 at 10:16:21AM +0200, Kai Blin wrote:
> >>-----BEGIN PGP SIGNED MESSAGE-----
> >>Hash: SHA1
> >>
> >>On 2012-10-12 09:38, Michael Adam wrote:
> >>
> >>Hi,
> >>
> >>>Ok, the main point is this:
> >>>
> >>>Change "review" from a matter of interest to a required step in
> >>>our software development.
> >>>
> >>>Hence, I am inclinend to change my proposal to:
> >>>
> >>>* we should _not_ allow a free push pass after any number of days
> >>>at all.
> >>Ok, as the one who works on an area that sees little interest at all
> >>until things break, how do I make sure I get my code in?
> >Just be a PITA for the reviewers. Send mails, call them,
> >start to use other means like hop on a train to Goettingen
> >with a LART tool :-)
> Of course I do understand the joke but in every joke there is a part
> of truth, so what about people in .au or .us ? Should I knock on the
> door of Jeremy to get a review ?

You do know where I live :-).

> I don't think it's being fair to the developer to ask him to nag
> reviewers in order to get a review. I do think that within 3 days a
> reviewer should pop up to say: I want to review this and then maybe
> the reviewer gets a bit more time to do the review but I still do
> think (also) that it should be limited so that we keep on the
> dynamic and review never get stuck for weeks as it has been the case
> in the past for some voluntary reviews.

Look at the things that have been stuck for weeks (and I'm
mainly thinking of poor ddis's async RPC rewrite).

They're mainly *big* changes that require lots of work to
review.

But you know what, they're the things that *should* be
reviewed more thoroughly.

I haven't seen the case where a simple patch didn't get
a prompt ack.

Jeremy.


More information about the samba-technical mailing list