Using Gerrit for code review in KDE

[Martin Gräßlin]

On Saturday 13 September 2014 20:38:21 Eike Hein wrote:
> The argument "but you can still bypass Gerrit and push
> directly, this is just social etiquette" doesn't matter
> because the whole thing is about social etiquette. The
> ACLs we already have reflect our social etiquette, and
> that means we need to be careful and think smart about
> evolving it.
> 
> Personally I think social etiquette encouraging the use
> of review tools is fine. Social etiquette that entrenches
> some developers as special on those review tools is not.

Thanks for raising your concerns! I think they are very valid and I think you 
touched a topic which we need to solve in more general: better handling of 
maintainer pass-over and removing ACL where it's not needed (repo ownership 
comes to my mind which is one of the areas I assume you meant).

I also see that I should have elaborated more. I had written more and removed 
it before sending the mail.

With review board I was quite often in the situation that I wanted to easily 
encode "I like the presented solution, but do not know the code in question 
enough to give you a shipit". That's what a +1 is to me. On the other hand I 
was also in the situation that I wanted to know whether the review I got is 
from someone with knowledge to the code base. Currently a shipit means one as 
a developer has to do "research" whether the reviewer knows the code good 
enough. A +2 as "I really know that code" from a project core member would 
really help there. I have seen more than once that people not knowing the code 
good enough gave a shipit and it got pushed before people with the knowledge 
looked at it.

I also had new developers in mind with my comment. Lets assume we have two new 
developers and the one giving the other a shipit and the other pushes but the 
code is in well say not yet good enough. If the maintainer reverts afterwards 
it might mean that we have lost a possible new contributor. I don't know 
whether this happens so often that we have to care about, but given that we 
move to pre-commit review I prefer not having to think about reverting at all.

That said: I see a strong advantage in having the complete range from -2 to +2 
available for development and -2,+2 only available to core members of the 
given repository. Giving +2 for everyone results in having the same as the 
shipit state currently: it's no easily interpretable way to read how familiar 
the reviewer is with the project.

But you raised a good point. I think this needs more brainstorming and 
discussions on the community mailing list to find a good solution which keeps 
our core values. I think at the same time we should tackle the other already 
implemented ACLs. As long as that is not solved I agree that we shouldn't make 
too fast steps and not implement an ACL on gerrit.

Cheers
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140913/8ea2b034/attachment.sig>