"co-authored-by" tags in commits are ignored when collecting props

[jrfnl]

Description

Given:

• A PR which is opened for review in a repo using the props bot
• ... and the PR contains one or more commits which already credit contributors using the Co-authored-by: tags.
• ... and the Co-authored-by: tags follow the format as proscribed in the attribution page, i.e. Co-authored-by: githubusername <[email protected]>
• ... and the accounts mentioned are linked correctly (i.e. the link between w.org and GitHub has been established correctly prior to the commits being made)

I'd expect:

The props bot to include those people being acknowledged as co-authors to be included in the props list.

Instead this happened:

Only the committer is included in the props list, co-authors are ignored by the bot.

Version

v1

Workflow file

https://github.com/WordPress/wordpress-develop/blob/trunk/.github/workflows/props-bot.yml

Link

WordPress/wordpress-develop#6235

[desrosj]

Thanks for this @jrfnl!

Some rough thoughts on this:

• Co-authored-by trailers in commits are not currently parsed. Thinking this through, it seems that they should be parsed, but only when collecting an list of contributors for SVN because a full list is needed when committing to develop.svn.wordpress.org.
• If the intention is to merge the PR in Git, then the squash and merge message should include the commit messages for each commit in the PR and thus any Co-authored-by trailers from those commits. These would be parsed out with any that are added manually.
• If we want all of the Co-authored-by trailers to be included at the bottom of a merge commit message, then we'd need to advise maintainers to collect them manually. It does not seem like there is a way to configure GitHub to do this in some way.
• This may actually be preferred, though. For example, say there are 3 commits in a PR branch and contributor A is noted as a co-author in all of them. Contributor A also comments on the PR so is included in the merge commit list produced by the bot. This would result in them receiving 4 props for the PR instead of 1, which is traditionally how we handle that in Core SVN.

I think parsing commit messages and including any trailers for SVN is a good first step here. The Git aspects need more feedback.

[jeffpaul]

For cases where the PR will be merged in GitHub, it seems like those co-authors-on-commits would still appear in the gitlog and thus be properly credited. It would be nice to have those co-authors-on-commits gathered and presented in its own listing so that whoever is merging the PR could see that list and perhaps include them in the PR merge commit if they wanted or alternatively if those same accounts are also included in props bot listing then they could be ignored in the merge commit since they'd already be part of the gitlog history.

More importantly, and I think @desrosj gets at this, for cases where the PR is not merged into GitHub and is instead committed to SVN, then having those co-authors-on-commits referenced by the props bot for the SVN listing would be desired to ensure those folks are not missed on the SVN commit.

[jrfnl]

for cases where the PR is not merged into GitHub and is instead committed to SVN, then having those co-authors-on-commits referenced by the props bot for the SVN listing would be desired to ensure those folks are not missed on the SVN commit.

That is exactly the usecase for which I opened the issue.