the review process, and how we got to doing the review process

[Andrew Bartlett]

On Fri, 2013-09-13 at 06:24 +0200, Kai Blin wrote:
> On 2013-09-12 14:30, Simo wrote:
> 
> > For area that have little contribution this is even more important,
> > because the code is probably not very much exercised so bugs will
> > probably take a while before being reported, and because there are fewer
> > people familiar with the code so having the author available to explain
> > the patch while fresh in his mind, if necessary is even more important.
> > Also the less people familiar the easier it is that they are not around
> > if the bug hits at a critical moment.
> 
> Right, but for an area nobody cares about, the reviews will not be very
> deep. The patch that broke SWAT for non-root users was reviewed by a
> number of people and nobody caught this. I have the feeling that if I 
> had a patch for the core password check logic in smbd, the patch would 
> have gotten more attention.
> 
> >> TLDR; The current voluntary system works. Don't fix it if it ain't broken.
> >
> > It was proposed, this time, as a technical solution to someone
> > forgetting to add tags, but what you are saying is: even though it seem
> > we all agree on doing reviews and in effect all the last ~2700 patches
> > each had a review, you want to be able to go ahead and forcibly push
> > un-reviewed patches on the off chance that nobody will care at all for
> > weeks to review one of your patches for mysterious reasons.
> 
> Can we just get past this implication that it's only me being hysterical 
> here? Others have voiced similar concerns. I don't think it's a 
> coincidence that there seems to be a large overlap between the 
> developers who are paid to work on Samba and the people who don't think 
> mandatory reviews are a problem, and also a large overlap between the 
> hobby developers and the people who are concerned about this. Arguably, 
> if you look at commit stats, the former group doesn't need the latter, 
> and can afford to put up more hoops for contributors to jump through. I 
> guess that's for the team to decide.

Kai,

Can I give you my assurance that you are certainly not alone here.  I
feel very much the same way.  I feel the team has done this whole review
thing very poorly, imposing a process that works (well, apparently) for
those who proposed it, and constantly irritates others, like yourself
and myself.  I am similarly frustrated that my concerns seem to be
brushed off, but also that I'm consistently unable to express them
clearly. 

I also see it as far more than the specifics and mechanics, the way this
was decided was a new low for the team in my view.  The fact that it
imposes a particularly high burden on remote, new and occasional
contributors is particularly poor. 

I certainly agree that the lack of certainly in the process is a big
issue.  For me it really messes with my head having this open-ended
process into which an even trivial patch is submitted, in the hope that
someone will review it, because it removes the finality, the knowing
that this particular piece of work is done, and I can now think about
something else.  I can imagine that would be particularly frustrating if
I only had occasional time to deal with Samba, as you would never know
when a particular task might be done!

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org