Code review required for commits - Discuss.
obnox at samba.org
Fri Oct 12 03:00:09 MDT 2012
On 2012-10-12 at 18:43 +1100, Andrew Bartlett wrote:
> On Fri, 2012-10-12 at 09:38 +0200, Michael Adam wrote:
> > Hi Andrew,
> > On 2012-10-12 at 18:18 +1100, Andrew Bartlett wrote:
> > >
> > > As Ira said before, and I tend to agree with him on this much, if nobody
> > > is interested after 3 business days, then nobody is interested.
> > > Stretching this process out for a week is only going to cause
> > > aggravation for no additional benefit.
> > 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.
> Then, under those terms, I must totally and unquestionably oppose this
Interestingly it was _your_ comment that made me rethink.
I take it you are afraid that your productivity will drop if
your patches need review. I don't think that this is the case.
If your patches are good, you hand them off for review and
someone else will push, maybe you have to ask a second time,
but so what. No real additional effort. If your patch was
buggy, and this is caught by the reviewer, the good thing is
that the fixing work is done before pushing to master and
not afterwards. The work is needed anyways...
The important thing is that you can continue to be productive
while your patches are not yet in master.
You simply work on your own branch.
You rebase your branch regularly while patches gradually
end up in master.
This is also what many of us do.
I can especially mention Metze, who does this all the time
(and our collaborative work on the durable handles).
And you'll have to consent (looking at ohloh) that metze
is not really less productive than you are.
Of course, doing rebases can be painful at times, but I think
it is worth the effort. And the main point is that your
productivity need not suffer when working on your own branches!
So, please, rethink your -1.
Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 206 bytes
Desc: not available
More information about the samba-technical