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
signature.asc
Description: This is a digitally signed message part.
