Reformat the Props Bot comment #21

desrosj · 2024-01-18T18:04:35Z

What?

This reformats the comment left by the bot in a few ways:

The SVN Props: style attribution line is now included as an option in the comment.
The headers for the Co-Authored-By style comment are removed.
The Unlinked Contributors are now listed before the Co-Authored-By ones.
There is significantly more context in the comment around what's going on, how to appropriately use the information in the comment, or learn more.
Unlinked contributors are now pinged as part of the comment with a request to connect their accounts.

Fixes #4, #8, #12, and #17.

This reformats the comment left by the bot in a few ways: - The SVN `Props:` style attribution line is now included as an option in the comment. - The headers for the `Co-Authored-By` style comment are removed. - The Unlinked Contributors are now listed before the `Co-Authored-By` ones. - There is significantly more context in the comment around what's going on, how to appropriately use the information in the comment, or learn more. - Unlinked contributors are now pinged as part of the comment with a request to connect their accounts.

github-actions · 2024-01-18T18:04:51Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @desrosj-bot.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core SVN

If you're a Core Committer, use this list when committing to wordpress-develop in SVN:

Props: desrosj, desrosjbot, jeffpaul, jorbin, peterwilsoncc, gziolo, joemcgill, pento.

GitHub Merge commits

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: desrosj-bot.

Co-Authored-By: desrosj <desrosj@git.wordpress.org>
Co-Authored-By: desrosjbot <desrosjbot@git.wordpress.org>
Co-Authored-By: jeffpaul <jeffpaul@git.wordpress.org>
Co-Authored-By: aaronjorbin <jorbin@git.wordpress.org>
Co-Authored-By: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-Authored-By: gziolo <gziolo@git.wordpress.org>
Co-Authored-By: joemcgill <joemcgill@git.wordpress.org>
Co-Authored-By: pento <pento@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the core handbook.

desrosjbot

This is a test review.

desrosj-bot · 2024-01-18T18:12:48Z

This is a comment from an account that is not linked to a WordPress.org profile.

It's not clear if this is necessary or will results in missed props currently.

desrosj · 2024-01-18T20:26:08Z

I'm pretty happy with where this is at. Tagging @aaronjorbin and @jeffpaul for a review since they both provided feedback to the message contents and phrasing.

jeffpaul

Minor text/link update but not a blocker for merge, THIS IS AWESOME THANK YOU FOR LEADING THIS!!!

dist/index.js

src/github.js

aaronjorbin · 2024-01-18T20:54:07Z

I agree with Jeff Paul. Minor suggestions above, but I'm 👍 as a whole.

peterwilsoncc

Minor suggestion but mainly testing the bot.

peterwilsoncc · 2024-01-18T22:40:40Z

src/contribution-collector.js

 						return;
 					}

-					return `Co-Authored-By: ${username} <${dotOrg}@git.wordpress.org>`;
+					return contributorLists['coAuthored'].push( `Co-Authored-By: ${username} <${dotOrg}@git.wordpress.org>` );


Is it possible to obtain the GitHub no-reply email addresses via the WP API you're using, ie in the format ID+username@users.noreply.github.com? This will allow co-authored commits to show up without people needing to associate it with their account. As an example, mine is 519727+peterwilsoncc@users.noreply.github.com

We can, but because our ultimate goal is to give credit in the Credits API, I chose to use the user@git.wordpress.org format. I also didn't love the idea of hard coding personal emails in the commit messages if someone has that setting turned off. I don't recall if the anonymous format worked even if you had that disabled. I could test if there's a strong preference to use that format instead.

The other thought I had was for consistency with how committers are attributed credit from the WordPress/wordpress-develop mirror. When a contributor is granted commit access, they are told to add the @git.wordpress.org format to their account, as that's the email given to the commit author. A "pie in the sky" idea that I'll investigate once we get this initial round settled is to include Co-Authored-By trailers for everyone that receives Props in an SVN commit. My thinking there is that the same format as the committer would make sense.

By including unlinked contributors in the commit message, though, we can easily extract their GH slug from the list and look in the .org database for any contributors that have connected their account since any of their contributions were merged.

There's some weirdness to detail around those emails as noted on that page:

If you created your account on GitHub.com after July 18, 2017, your noreply email address for GitHub is an ID number and your username in the form of ID+USERNAME@users.noreply.github.com. If you created your account on GitHub.com prior to July 18, 2017, and enabled Keep my email address private prior to that date, your noreply email address from GitHub is USERNAME@users.noreply.github.com.
In my research from a year or so ago, I couldn't find a way to figure out which format a specific user has. But in recent testing, I found that both formats worked, regardless of the format it tells you you have.

Another layer to that is on the Change your username page:

If you've been using a GitHub-provided private commit email address, whether or not your commit history will be retained after an account rename depends on the format of the email address. Git commits that are associated with your GitHub-provided noreply email address won't be attributed to your new username and won't appear in your contributions graph, unless your noreply email address is in the form of ID+USERNAME@users.noreply.github.com. Older versions of the noreply email address that do not contain a numeric ID will not be associated with your GitHub account after changing your username.

So we'd always need to use the ID+USERNAME format just in case someone changes their username. On the other hand, WP.org usernames cannot be changed.

Sorry for the brain dump, but this was a good chance to document some of my thinking/findings.

Given the goal here is to improve the breadth of folks properly credited from GitHub repos back to WordPress core major releases and to streamline the major release credits gathering process, the approach that @desrosj has here makes sense. If the goal was to ensure all GitHub users were properly credited on PRs, then using the format @peterwilsoncc noted would likely be the best approach.

Been thinking about this the last few days.

I think I do prefer using all .org usernames, but it could be reasonable to use the GH format email for any contributor that's not linked to a .org one.

We can still parse out their GH user and look them up in the .org API, and they'd receive attribution on GH from their anonymous email. But that also removes some of the incentive to connect their account in the first place.

@desrosj Does the WP.org integration add username@git.wordpress.org to a users profile when they link their accounts or is that a step the contributor would need to do to get recognition on GitHub?

Currently, they will need to add it manually. I plan to include this in the announcement post, and the documentation. Based on this comment , though, it doesn't seem that our authentication request currently allows us to modify your profile at all. We'd need to add that to the request, and contributors with pre-existing connections would need to re-approve the connection with the new permissions.

I was talking this through with @dd32 last night. Wanted to document our discussion. At first he did not like using @git.wp.org emails, but after some discussion, this was how he summarized his thoughts:

On a GitHub-first repo, it makes sense to use @…github.com emails

On a SVN repo, it makes sense to use username

On a SVN -> Git sync, it makes sense to append Co-Authored-by: …@git.wordpress.org (But who knows if we wanna do that)

On a SVN -> Git -> GitHub sync, We can’t modify the commit message of ^ so meh, i guess it makes sense to use @git.w.org

Because of ^, it makes sense that on a GitHub-First repo, we could just use @git.w.org for consistency.

It's worth noting that in that list, there are currently no SVN -> Git syncs that do not make it over to GitHub.

Some other takeaways from our discussion:

Even though we're heavily using GitHub at this point, there's always a non-0 chance that as a project we decide to use something other than GitHub (ie. GitLab). If this does happen, then the ID+username@github emails would be out of place. We could likely add a way for the tool to support these, but using wp.org ones would be somewhat tool-agnostic.

There's currently no ability to validate that you own the @git.wp.org email that represents you. Adding the ability to verify this through email will likely be appropriate to avoid any problems.

I've gone and created #28 for changing the unlinked contributor list to use GH emails. I think that makes sense, but figured it would be good to tackle separately.

Co-authored-by: Aaron Jorbin <aaronjorbin@users.noreply.github.com>

desrosj · 2024-01-23T20:20:48Z

I'm going to merge this. I hope I'm reading it right, but based on your original comment @peterwilsoncc, I don't think any of that discussion is a blocker. If I'm wrong, please feel free to create an issue for anything that is unaddressed and we can continue discussing!

jeffpaul

Evolving that sad, grey checkmark into a more powerful, capable green checkmark. Checkmark, I choose you!

aaronjorbin · 2024-01-23T21:15:18Z

src/github.js

+
+		commentMessage += contributorsList['coAuthored'].join("\n") +
+		"\n```\n\n" +
+		"**To understand the WordPress project's expectations around crediting contributors, please [review the core handbook](https://make.wordpress.org/core/handbook/).**\n";


I added ticket #29 to replace this link, but it's not a blocker.

peterwilsoncc

'm going to merge this. I hope I'm reading it right, but based on your original comment @peterwilsoncc, I don't think any of that discussion is a blocker.

No, not a blocker.

joemcgill · 2024-01-24T01:10:16Z

I have not gotten a chance to dive deeply into the implementation here, but I do really like this idea and think it will greatly improve the DX for people committing changes to any of our repos.

I’m curious if there’s an ability to sync any related comments from a linked Trac ticket when this is used in the Core SVN repo, since most of the time it takes a bit of effort to read through a ticket to determine who to add to a props list along with anyone who contributed directly on the PR.

No need to answer that question before merging this, just more for me to better understand the end goal here.

pento · 2024-01-24T04:38:03Z

Very excited to see this happening!

I'm definitely in favour of using the @git.wordpress.org format. There have been a bunch of good reasons listed for this, but ultimately it really only needs one excellent reason: it ensures that WordPress' commit history is service agnostic. In my opinion, this reason beats anything else.

Much as we did with Slack email verification so many years ago, setting up limited email forwarding for @git.wordpress.org addresses, so that GitHub can verify ownership of that account, would close the loop here.

If it turns out that lots of people don't add their @git.wordpress.org address to their GitHub profile, I think it'd be worth exploring having the WordPress.org Profiles app request permission to be able to update email addresses, so it could be done automagically. I think we can all imagine the controversy that might trigger, however, so it'd be nice if GitHub supported optional permissions, allowing people to decide if they wanted to grant that access. If only we knew anyone who worked for GitHub who could push for that... 😉

jeffpaul · 2024-01-24T14:28:46Z

Much as we did with Slack email verification so many years ago, setting up limited email forwarding for @git.wordpress.org addresses, so that GitHub can verify ownership of that account, would close the loop here.

Yes, very much in favor of that. What's the best way for us to coordinate within the project to make that happen?

desrosj · 2024-01-24T15:03:58Z

There was a request put in earlier today for this on the Make Systems blog.

desrosj · 2024-01-24T16:33:14Z

I’m curious if there’s an ability to sync any related comments from a linked Trac ticket when this is used in the Core SVN repo, since most of the time it takes a bit of effort to read through a ticket to determine who to add to a props list along with anyone who contributed directly on the PR.

@joemcgill Great idea! That shouldn't be too hard to implement. Each ticket has an RSS feed that we can pull in and process. I created #30 to track this!

desrosj · 2024-01-24T19:14:35Z

Thanks everyone for the great work and feedback! I'm going to merge this in the interest of keeping things moving. We can follow up in the spin-off issues that have been created.

gziolo · 2024-02-01T09:17:02Z

src/github.js

+				"Contributors, please [read how to link your accounts](https://make.wordpress.org/core/2020/03/19/associating-github-accounts-with-wordpress-org-profiles/) to ensure your work is properly credited in WordPress releases.\n\n";
+		}
+
+		commentMessage += "## Core SVN\n\n" +


@desrosj, would it be possible to make this configurable? While Core SVN makes perfect sense in WordPress develop, the GitHub Merge isn't that useful. Then, when contributing to Gutenberg, it exactly the other way around. So while the default could be as is, what if you could pass the setting when defining the GitHub workflow?

svnProps: true gitCoAuthored: true

Could you make an issue for this on the @WordPress/props-bot-core repo?

Pleasure: #38 :)

Some cleanup and typo fixes.

9bfdbf4

desrosjbot reviewed Jan 18, 2024

View reviewed changes

Only show Unlinked contributor: when there's 1 or more.

3c9cb48

desrosj added 8 commits January 18, 2024 13:15

More formatting corrections.

6df5feb

Remove some debug code.

5f7e6b7

Don't recommend the merging contributor remove themselves.

bd1c7ce

It's not clear if this is necessary or will results in missed props currently.

Breaking out the pinking shears.

a2b9c88

Add note about props-bot label.

8c631f2

Remove stray semi-colon.

ee2caed

More refinement of phrasing.

a74a6c2

Avoid including a subscript in comment.

b212ae0

desrosj marked this pull request as ready for review January 18, 2024 20:25

desrosj requested review from aaronjorbin and jeffpaul January 18, 2024 20:25

desrosj added 2 commits January 18, 2024 15:26

Remove extra core.debug().

03d60a3

Remove correct extra debug line.

93260ce

jeffpaul approved these changes Jan 18, 2024

View reviewed changes

dist/index.js Show resolved Hide resolved

src/github.js Outdated Show resolved Hide resolved

peterwilsoncc reviewed Jan 18, 2024

View reviewed changes

desrosj and others added 2 commits January 18, 2024 19:03

Change phrasing.

5ca2d5e

Co-authored-by: Aaron Jorbin <aaronjorbin@users.noreply.github.com>

Rebuilding dist.

dae2833

This was linked to issues Jan 19, 2024

Add a comment suggesting unconnected users create/link their wp.org account #8

Closed

Don't include a contributor section in the comment when empty #12

Closed

Remove empty lines between contributors #17

Closed

desrosj mentioned this pull request Jan 19, 2024

Don't display section headers #18

Closed

Tweak messaging.

b5e878f

Update built files.

9d62018

desrosj self-assigned this Jan 23, 2024

Merge branch 'trunk' into update/comment-format

124373b

jeffpaul approved these changes Jan 23, 2024

View reviewed changes

aaronjorbin approved these changes Jan 23, 2024

View reviewed changes

aaronjorbin reviewed Jan 23, 2024

View reviewed changes

peterwilsoncc approved these changes Jan 23, 2024

View reviewed changes

desrosj merged commit e722687 into trunk Jan 24, 2024
10 checks passed

desrosj deleted the update/comment-format branch January 24, 2024 19:16

gziolo reviewed Feb 1, 2024

View reviewed changes

gziolo mentioned this pull request Feb 1, 2024

Allow configuring the details included in the comment #38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reformat the Props Bot comment #21

Reformat the Props Bot comment #21

desrosj commented Jan 18, 2024

github-actions bot commented Jan 18, 2024 •

edited

desrosjbot left a comment

desrosj-bot commented Jan 18, 2024

desrosj commented Jan 18, 2024

jeffpaul left a comment

aaronjorbin commented Jan 18, 2024

peterwilsoncc left a comment

peterwilsoncc Jan 18, 2024

desrosj Jan 19, 2024 •

edited

jeffpaul Jan 22, 2024

desrosj Jan 22, 2024

peterwilsoncc Jan 23, 2024

desrosj Jan 23, 2024 •

edited

desrosj Jan 23, 2024

desrosj commented Jan 23, 2024

jeffpaul left a comment

aaronjorbin Jan 23, 2024

peterwilsoncc left a comment

joemcgill commented Jan 24, 2024

pento commented Jan 24, 2024

jeffpaul commented Jan 24, 2024

desrosj commented Jan 24, 2024 •

edited by dd32

desrosj commented Jan 24, 2024

desrosj commented Jan 24, 2024

gziolo Feb 1, 2024

desrosj Feb 1, 2024

gziolo Feb 1, 2024

Reformat the Props Bot comment #21

Reformat the Props Bot comment #21

Conversation

desrosj commented Jan 18, 2024

What?

github-actions bot commented Jan 18, 2024 • edited

Unlinked Accounts

Core SVN

GitHub Merge commits

desrosjbot left a comment

Choose a reason for hiding this comment

desrosj-bot commented Jan 18, 2024

desrosj commented Jan 18, 2024

jeffpaul left a comment

Choose a reason for hiding this comment

aaronjorbin commented Jan 18, 2024

peterwilsoncc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desrosj Jan 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desrosj Jan 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desrosj commented Jan 23, 2024

jeffpaul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc left a comment

Choose a reason for hiding this comment

joemcgill commented Jan 24, 2024

pento commented Jan 24, 2024

jeffpaul commented Jan 24, 2024

desrosj commented Jan 24, 2024 • edited by dd32

desrosj commented Jan 24, 2024

desrosj commented Jan 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 18, 2024 •

edited

desrosj Jan 19, 2024 •

edited

desrosj Jan 23, 2024 •

edited

desrosj commented Jan 24, 2024 •

edited by dd32