As brought up in a Discourse committers topic, the devguide could be more clear about Pull Request attribution on GitHub.

Specifically, in the fourth paragraph of "Handling Others' Code", the Co-authored-by: feature of GitHub is mentioned, but the use case of it is not defined.

Since GitHub will automatically attribute the primary author of the PR when using "squash and merge" or the "automerge", the Co-authored-by: is useful to add just prior to merging for anyone that significantly contributed to the PR, either via review or direct suggested changes. When using the GitHub UI to directly commit a suggested change to the PR, a Co-authored-by: line is automatically added to the commit message. Otherwise though, it has to be done manually for people other than the author to be attributed for contributing to the PR.

With this being a rather substantial part of the workflow for GitHub PRs, it seems to be worthwhile to mention when Co-authored-by: should be used rather than mentioning the feature without elaborating further.

In addition to the "Co-authored-by," the "Patch by" instructions also need to be clarified. Like, one sentence says this:

Third, ensure the patch is attributed correctly with the contributor’s name in Misc/ACKS if they aren’t already there (and didn’t add themselves in their patch) and by mentioning “Patch by ” in the Misc/NEWS.d entry and the check-in message.

For the second part, it seems like "Patch by" is redundant in the commit message if they're already the author of the first commit. It should also clarify whether "Patch by" should be added to the NEWS entry if the Git history already has them as the primary author.

I would also check more widely before saying that reviewers should be credited in "Co-authored-by." I had never heard of that suggestion before today.

I would also check more widely before saying that reviewers should be credited in "Co-authored-by." I had never heard of that suggestion before today.

Specifically, I meant that if a reviewer suggested a specific change, and it was added to the PR. In the GitHub UI when directly committing a suggestion, it automatically adds the person who made the suggestion to the "Co-authored-by:". So, I think it makes sense to do this manually if the author directly adds a change suggested by the reviewer. In a case where the reviewer makes a suggestion that does not get incorporated into the PR, it wouldn't make sense to include them in the "Co-authored-by:".

But you're right that we should verify this more widely before suggesting it in the devguide, I'll try to make a separate thread to get feedback on this when I can find the spare cycles to do so.

Specifically, I meant that if a reviewer suggested a specific change, and it was added to the PR.

I don't know. That doesn't seem right to me. That would basically mean that anytime a reviewer made a suggestion on a PR that the author agreed to, they'd need to be added as a co-author. That seems like a significant broadening of the definition. Like, if you think about someone who writes a book, the book's editor might do a ton of work on making suggestions, but they're not listed as a co-author of the book.

On this point,

In the GitHub UI when directly committing a suggestion, it automatically adds the person who made the suggestion to the "Co-authored-by:".

If the suggestion is small, I would actually suggest the reverse and remove the Co-author attribution. (That's what I did when I made some small changes to someone else's PR before committing.) Just because GitHub's UI does something, I don't think we need to adopt that.

That would basically mean that anytime a reviewer made a suggestion on a PR that the author agreed to, they'd need to be added as a co-author. That seems like a significant broadening of the definition. Like, if you think about someone who writes a book, the book's editor might do a ton of work on making suggestions, but they're not listed as a co-author of the book.

That's a great point, I honestly hadn't thought about it that way. Perhaps then it should be based more on the significance of the suggestion(s) with respect to the entire PR. So, if a suggestion was made that had a fundamental impact on the overall feature, bug fix, behavior change, etc. a Co-authored-by: should be considered, but not for smaller ones (such as Misc/NEWS fixes, typos, and other minor issues).

For my own authored PRs, I've had a tendency to be more liberal with regards to giving Co-authored-by credit more recently when others provide good suggestions that have an impact on the PR. I find that it somewhat helps to encourage collaboration, and in general I've found that reviews are frequently underrated in significance (especially from non-core devs), so I'm glad to encourage it as best I can.

That being said, I can definitely understand the issue that it may be unfairly giving away too much credit to minor suggestions. This definitely seems like something we need to have a wider discussion about with regards to appropriate usage.

If the suggestion is small, I would actually suggest the reverse and remove the Co-author attribution. (That's what I did when I made some small changes to someone else's PR before committing.) Just because GitHub's UI does something, I don't think we need to adopt that.

I'm not sure if this is something that any other core developers already do, but after some consideration I agree that it would probably make sense to manually remove the Co-authored-by for minor suggestions when GitHub automatically adds it.

I may have given a bit too much weight to the fact that it happens to be a feature, and not adequately considered that a different policy might make more sense for us. Thanks for the feedback, I mostly agree with the points you made.

I think it depends on the magnitude of the change. I just reworded a one-sentence news item and removed the co-author note. On the other hand, I have often so thoroughly revised a PR that in the past I added myself as co-author.