Gerrit needs to let non-committers give Code-Review +1 |
|||||||
Issue descriptionWhile trying to land crrev.com/c/696190 the reviewer and I hit some odd failures. Our best guess is that these are because the reviewer is an OWNER but not a committer, and gerrit doesn't support that. That is, the reviewer sees "Code-Review You don't have permission to edit this label." When Rietveld was the review tool this reviewer was able to give lgtm (but, of course, could not land changes without the intervention of a committer). The reviewer also can't trigger a dry run but that may be WAI. I don't know how many owners are not committers, but presumably we either need to remove them or support them.
,
Oct 5 2017
,
Oct 5 2017
OWNERS *must* be committers, by policy: https://chromium.googlesource.com/chromium/src/+/master/docs/code_reviews.md#OWNERS-files
,
Oct 6 2017
That doc also says: > Seldom-updated directories may have exceptions. Directories in third_party should list those most familiar with the library. Which exactly matches the related case.
,
Oct 20 2017
Aaron, please see my previous comment.
,
Oct 20 2017
That exception is for requirements like "Have committed or reviewed substantial work to the affected directory within the last 90 days" and "Have submitted a substantial number of non-trivial changes to the affected directory", which are obviously not true for a third-party library which is barely- or un-modified and seldom updated. That exception does not apply to the requirement to be a committer.
,
Oct 24 2017
agable@, it is not clear to me at all that this exception does not apply to that requirement in particular, but applies to others. Certainly, it's not written anywhere in that document. Can you update the doc to make it clear what this exception apply to? > "Have submitted a substantial number of non-trivial changes to the affected directory", which are obviously not true for a third-party library which is barely- or un-modified and seldom updated. The reason I am an owner of protobuf is precisely because updating protobuf requires some non-trivial work and expertise.
,
Oct 24 2017
...and I've been doing protobuf updates in the past (sent the comment to early).
,
Nov 1 2017
Aaron, ping.
,
Nov 4 2017
Yep, happy to update the doc to make the situation clearer: https://canary-chromium-review.googlesource.com/c/chromium/src/+/754381 There are two good paths forward here that I see: 1) Become a committer 2) Pull third_party/protobuf into a separate repo, which can have its own committers list and you in the OWNERS file, and then simply submit DEPS rolls when appropriate. Please recognize that the reason we have to restrict the ability to CR+1 changes is that the ability to do so is equivalent to direct Write access to the repository, albeit with a better audit trail. We are an open source project, and cannot grant that permission broadly, unlike Google's internal codebases which can grant wide-reaching access to all employees precisely because they're employees. And because we value being an open-source project, we don't even grant CR+1 permission to all Googlers: employees have to earn the right to be a committer just like everyone else.
,
Nov 6 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by brucedaw...@chromium.org
, Oct 5 2017