I wasn't aware of this history, thanks for explaining! In most Apache projects I contributed to, the list of things that are stated in "reviewed by" are implied in a committer committing the patch. Reviewers are there to help the committer make the decision (thats why I sometimes mention "@ewencp please take a look and let me know what you think" in a JIRA), but the responsibility is on the committer, not the reviewers. Reviewing by community members is considered a valued contribution, and therefore "acknowledged" in the commit message (or sometimes in the JIRA).
If the intent is to avoid surprises (which is a good idea), once we decide on how we do things we can clarify in the contributor guide (even if the decision is to keep current method, we should document it) On Wed, Jul 29, 2015 at 5:33 PM, Ismael Juma <ism...@juma.me.uk> wrote: > On Wed, Jul 29, 2015 at 7:38 PM, Gwen Shapira <gshap...@cloudera.com> wrote: > >> I guess we see the "reviewer" part with different interpretations. >> > > Yes. As you know, Git was created for and initially used by the Linux > Kernel. As such they were very influential in conventions, terminology and > best practices. This is what their documentation states: > > "By offering my Reviewed-by: tag, I state that: > > (a) I have carried out a technical review of this patch to > evaluate its appropriateness and readiness for inclusion into > the mainline kernel. > > (b) Any problems, concerns, or questions relating to the patch > have been communicated back to the submitter. I am satisfied > with the submitter's response to my comments. > > (c) While there may be things that could be improved with this > submission, I believe that it is, at this time, (1) a > worthwhile modification to the kernel, and (2) free of known > issues which would argue against its inclusion. > > (d) While I have reviewed the patch and believe it to be sound, I > do not (unless explicitly stated elsewhere) make any > warranties or guarantees that it will achieve its stated > purpose or function properly in any given situation." > > This is a common interpretation when the word reviewer or reviewed-by is > used in a Git commit. If we mean something else, maybe it's better to use a > different word. > > What are the benefits you see of formalizing who gets mentioned as reviewer? >> > > * Consistency > * Easier for new committers if there's a clear guideline > * Avoid surprises for people who comment on PRs. Now that we are accepting > pull requests via GitHub, it is more likely that people will comment on > pull requests as they see it in their news feed (the repository has more > than 300 watchers and this number is likely to rise); many/most of them > won't be doing a proper review. > > In any case, if the committers don't think this is an issue, we can > continue as it is and see if anyone else complains. Each community is > different after all. > > Best, > Ismael
