ianlancetaylor
commented
1 year ago I'm not sure this needs to be a proposal.
I tend to think that a +1 vote should mean "not obviously wrong" and also "does not introduce a security vulnerability." I agree that that is a higher bar, but it seems to me that it's not a much higher bar. Most changes obviously do not introduce a vulnerability. And if you aren't sure whether it does or not, that seems like a reasonable indication that it's not a good change for you to review.
As a separate comment, we should explicitly discourage a +1 vote after the change has been submitted. Those do happen from time to time, and serve no useful purpose. I'm happy to encourage people to read old changes, and comment on them if they see something, but not to vote on them.
bcmills
Issue
There are at least two published definitions in the Go project for the meaning of +1 on a Gerrit review, with the more recent definition on the GerritAccess wiki page seeming to set a higher bar than the earlier and still published definition on the Gardening wiki page.
Details
Gardening page The Gardening wiki page currently includes this (added by Brad in early 2017):
GerritAccess page The GerritAccess wiki page currently includes this (introduced by Russ in 2022 (as part of the compliance and supply chain security code review requirement changes):
The second sentence in that quote (especially the use of "confident" and "any sort of") reads to me as a substantially higher bar than "not obviously wrong". (One example is a non-obvious performance problem that could turn into a denial-of-service security issue, which can be tough for a casual contributor to feel confident is not present).
Most of the second sentence was taken directly from the older and now replaced language describing Trust+1:
Contribution Guide There is another mention of +1 in the Contributing Guide (which looks to be purposefully making a higher-level statement):
Suggestion
Ideally, the "not obviously wrong" meaning would be reaffirmed. It could be strengthen with "and does not contain a clearly inappropriate code change", or something similar.
If that sets the bar too too low for compliance and security reasons, then perhaps the definition could be split between what a +1 means for a Googler on the core Go team vs. the meaning for an external contributor giving a +1.
Alternatively, perhaps a new "triaged" vote or similar could be introduced with a meaning closer to "not obviously wrong", or some other solution.
Rationale
It is already somewhat intimidating to think about contributing to a project as large and successful as Go.
Beginning some form of interaction with Gerrit is a great on-ramp to becoming a more active contributor, whether it starts by lurking, or just subscribing to some interesting CLs via the star icon, or making a drive by comment to another new contributor, or actually reviewing a CL.
The prospect of being able to do something as useful as give a +1 is a draw for someone to register for Gerrit, even if they then only use their Gerrit registration for a while for more passive things like starring CLs. (For me personally, I lurked on Gerrit for a long-ish time before I ever registered there, but a "gardening" pitch I heard from Brad in a talk or podcast is a large part of what got me over the hurdle of registering).
Once someone has interacted with some CLs (comments, or just starring), the ensuing notifications spread out over time then offer multiple chances for something interesting to arrive in the potential contributor's inbox when they are not otherwise busy.
There are material benefits for the potential contributor as well when they engage in Gerrit, including improving their own development skills by seeing the rigorous processes of the Go project for code reviews and testing and its approach to software engineering. A deeper understanding of the internals of things like the runtime, compiler, and tools helps in their day-to-day use of Go. If they do later choose to more actively contribute, that can help career development.
I'm hoping the Go project continues to encourage triage-level reviews from outside contributors. It is a true virtuous circle, where the benefits integrated over time are substantial even if point-in-time benefits are sometimes hard to observe (e.g., the confusing bug report that was never written because someone had deepen their understanding of Go after starring & following a handful of CLs).