Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch Props Bot to using git flavored props. #182

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

desrosj
Copy link
Contributor

@desrosj desrosj commented Feb 6, 2025

No description provided.

@desrosj desrosj self-assigned this Feb 6, 2025
Copy link

github-actions bot commented Feb 6, 2025

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props desrosj.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@desrosj desrosj merged commit f2ad62f into WordPress:master Feb 6, 2025
29 checks passed
@desrosj desrosj deleted the adjust/props-bot branch February 6, 2025 17:41
@jrfnl
Copy link
Member

jrfnl commented Feb 6, 2025

Why is the Props bot being added in this project in the first place ? The changelog for the plugin isn't being compiled in the same way, now is it ?

@desrosj
Copy link
Contributor Author

desrosj commented Feb 6, 2025

Sorry, I'm not sure I follow.

As far as I know, the GitHub option to generate release notes will use the GitHub usernames for contributors. There are also no contributor names in the readme.txt.

The benefit of Props Bot is that it uses the [email protected] style Co-authored-by trailers making it easier to parse logs and give credit through any .org posts or APIs and also gives attribution to anyone who just comments, reviews, or reports issues. As previously configured, GH would only includes people that have made commits.

@jrfnl
Copy link
Member

jrfnl commented Feb 6, 2025

As far as I know, the GitHub option to generate release notes will use the GitHub usernames for contributors.

That's neither here nor there as that option is not used by this repo.

There are also no contributor names in the readme.txt.

Exactly. Not in any of the previous releases, nor in the changelog in the readme.txt.

In other words, this plugin has it's own release process in which it is not customary to "prop" anyone, committer, contributor or otherwise.

The benefit of Props Bot is that it uses the [email protected] style Co-authored-by trailers making it easier to parse logs and give credit through any .org posts or APIs and also gives attribution to anyone who just comments, reviews, or reports issues. As previously configured, GH would only includes people that have made commits.

I don't agree as this is a separate repo for which that principle is not used in the first place,

  • The [email protected] trailers have no meaning as the repo is not managed via Trac.
  • There are no logs being generated.
  • This causes spam approvals from people trying to get some "props" for doing nothing. (there are of course genuine approvals too, but that's a different matter)
  • It causes a stream of useless GH notifications (and emails if enabled) for anyone watching this repo.

@jrfnl
Copy link
Member

jrfnl commented Feb 6, 2025

Oh and IMO it is also an abuse of the co-authored-by tag. That should only be used for people who have touched the code, like to give credit to someone who originally wrote the code or wrote the code which inspired the commit, not for people who didn't "author" anything.

@peterwilsoncc
Copy link

I don't agree as this is a separate repo for which that principle is not used in the first place,

That may have been the case, but adding the props bot allows that to change. Over the last few years, the project has been trying to improve how and when props are given because it's always been important; there have been shortcomings but the goal that's what the props bot, including documentation authors, testers, etc has always been about.

Oh and IMO it is also an abuse of the co-authored-by tag.

I don't entirely disagree but it doesn't appear that GitHub recognizes other standardized annotations as contributors based on a few pushes to my own site's repo.

Screenshot 2025-02-07 at 8 42 45 AM

If GitHub introduces support for these annotations and recognizes them as contributors, the props bot can always be updated to use them. It would be quite the enhancement but hopefully achievable.

@jrfnl
Copy link
Member

jrfnl commented Feb 6, 2025

That may have been the case, but adding the props bot allows that to change. Over the last few years, the project has been trying to improve how and when props are given because it's always been important; there have been shortcomings but the goal that's what the props bot, including documentation authors, testers, etc has always been about.

I fully support giving credit where credit is due, but that still doesn't justify forcing the Props bot on this repo.

This is a low traffic repo AND it is a git-based repo and git usernames can be used for giving people credits in the releases.

The fake wordpress.org email addresses have no value.

Oh and IMO it is also an abuse of the co-authored-by tag.

I don't entirely disagree but it doesn't appear that GitHub recognizes other standardized annotations as contributors based on a few pushes to my own site's repo.

If GitHub introduces support for these annotations and recognizes them as contributors, the props bot can always be updated to use them. It would be quite the enhancement but hopefully achievable.

While GitHub recognizing those tags would be nice, I don't think it's relevant at all if the Props bot is being used. In that case, the first thing needed would be for the Props bot to start actually recognizing those tags in every single way.

As things are currently, the props bots doesn't even recognize and actually removes REAL co-authored-by tags, effectively REMOVING credit from people who deserve it. Also see: WordPress/props-bot-action#86

@desrosj
Copy link
Contributor Author

desrosj commented Feb 7, 2025

One of the goals I have with Props Bot is to make it easier to have a consistent attribution standard across all community maintained properties (which I would say includes this plugin). Personally, I think that we've failed the community by not being consistent with expectations and attribution when contributing across these projects.

When there is a release of a community maintained plugin, we should announce it in a post (probably on the Developer Blog with the changelog and a list of contributors celebrating their efforts like we do with Core releases. We should also expand the .org API to accommodate this, making the git.w.org emails useful to more easily parse the log and give attribution to all types of contributions so that we can come up with new ways to showcase who in the community is working on what.

As things are currently, the props bots doesn't even recognize and actually removes REAL co-authored-by tags, effectively REMOVING credit from people who deserve it. Also see: WordPress/props-bot-action#86

This causes spam approvals from people trying to get some "props" for doing nothing. (there are of course genuine approvals too, but that's a different matter)

While it's true that co-authored-by trailers in the PR's commit messages are not currently collected and included in the props list, they remain in the generated merge commit message when someone clicks the merge button. The maintainer merging the code can choose to leave these and the code will continue to be attributed to that person. A quick manual review of the list provided should always be done to remove any disingenuous or unproductive contributions and to add anyone missing.

It was my mistake for not including them for contributions to the test files when creating the PR for syncing the changes over.

I agree that it's not the most precise or truly intended use of co-authored-by, and that the notifications can be annoying but the previous process in place for releases was too time consuming and needed to be automated. Overall, it's not about whether there is a Trac association or not, it's about being able to more easily associate a change with a contributor's w.org account for attribution purposes (which for better or worse is how the project currently "officially" records contributions) and giving all types of contributions credit (which including co-authored-by does for someone's GH account). When users do not use the anonymous email or real name on GitHub (either through their GH settings or their local Git config), it's sometimes impossible to confidently make an association between the two locations without manual review, and even then it's not a great success rate. We found this with the Gutenberg before repository before Props bot was put in place.

For the notifications, perhaps we should append the props list at the very bottom of the PR description instead. The comment happens once per PR before being updated, and was a reminder to be mindful all types of contributions to the effort. We can and should constantly improve this to be more useful and less intrusive, but the bot met the requirements for what was trying to be done at the time.

If you feel this strongly about the bot not having a place in this repository, we can take it out. I've been trying to implement it in each repository as I work on them to make it easier to backfill contributions when we are able to expand the credits API. But not a hill I am willing to die on at this time.

@jrfnl
Copy link
Member

jrfnl commented Feb 7, 2025

@desrosj More than anything, what I feel strongly about is that the plans to change the release process for, and to introduce Props bot to, plugin repos should have been discussed publicly, either in an issue or via a Make post.

And yes, I detest the unnecessary noise props bot gives in notifications and email and find it unacceptable that it REMOVES credit when credit is properly given. Maybe those things should have been addressed before worsening the situation.

@desrosj desrosj mentioned this pull request Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants