Using Gerrit for code review in KDE
mgraesslin at kde.org
Sat Sep 13 20:34:45 BST 2014
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 181 bytes
Desc: This is a digitally signed message part.
More information about the kde-core-devel