+1 on Gwen¹s suggestion. Consider this as my thank you for all the reviews everyone has done in past and are going to do in future. Don¹t make me say thanks on every single commit. Introducing another process when the project has > 50 PR open pretty much all the time is not really going to help.
Thanks Parth On 7/29/15, 10:40 AM, "Gwen Shapira" <gshap...@cloudera.com> wrote: >My two cents: > >The jira comment is a way for the committer to say "thank you" to >people who were involved in the review process. It doesn't have any >formal implications - the responsibility for committing good code is >on the committer (thats the whole point). It doesn't even have >informal implications - no one ever went after a reviewer if a code >turned out buggy. > >I suggest: Leave it up to the committer best judgement and not >introduce process where there's really no need for one. > >Gwen > >On Wed, Jul 29, 2015 at 6:18 AM, Ismael Juma <ism...@juma.me.uk> wrote: >> Hi all, >> >> As a general rule, we credit reviewers in the commit message. This is >>good. >> However, it is not clear to me if there are guidelines on who should be >> included as a reviewer (please correct me if I am wrong). I can think >>of a >> few options: >> >> 1. Anyone that commented on the patch (in the pull request or Review >> Board) >> 2. The ones that have reviewed and approved the patch (+1, LGTM, Ship >> it, etc.) >> 3. A more sophisticated system that differentiates between someone >>who >> reviews and approves a patch versus someone who simply comments on >>aspects >> of the patch [1] >> >> On the surface, `1` seems appealing because it 's simple and credits >>people >> who do partial reviews. The issue, however, is that people (including >> myself) may not want to be tagged as a reviewer if they left a comment >>or >> two, but didn't review the change properly. Option `2` is still simple >>and >> it avoids this issue. >> >> As such, I lean towards option `2`, although `3` would work for me too >>(the >> additional complexity is the main downside). >> >> Thoughts? >> >> Best, >> Ismael >> >> [1] I don't think we should go this far, but the Linux Kernel is an >>extreme >> example of this with `Signed-off-by`, `Acked-by`, `Cc`, `Reviewed-by`, >> `Tested-by`, `Suggested-by`, `Reported-by`, `Fixes`, etc. More details >>in >> their documentation: >> https://www.kernel.org/doc/Documentation/SubmittingPatches >
