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

Improve conditions Props Bot runs under. #966

Merged
merged 1 commit into from
Feb 2, 2024
Merged

Conversation

desrosj
Copy link
Contributor

@desrosj desrosj commented Feb 2, 2024

Summary

This improves the logic controlling when Props Bot runs to:

  • not run for draft PRs.
  • not run when comments are deleted (this should hardly ever happen).
  • not run on closed PRs.

This improves the logic controlling when Props Bot runs to:
- not run for draft PRs.
- not run when comments are deleted (this should hardly ever happen).
- not run on closed PRs.
Copy link

github-actions bot commented Feb 2, 2024

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 SVN

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

Props: desrosj, westonruter, adamsilverstein.

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.

Co-authored-by: desrosj <desrosj@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>

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

@westonruter
Copy link
Member

  • not run when comments are deleted (this should hardly ever happen).

@desrosj What about in the case of spam drive-by comments? If they get deleted or hidden due to spam, would the bot purge the props properly?

@desrosj
Copy link
Contributor Author

desrosj commented Feb 2, 2024

@desrosj What about in the case of spam drive-by comments? If they get deleted or hidden due to spam, would the bot purge the props properly?

The next time it runs it would, yes. If the PR is ready to merge and there's still a spam account listed, you can manually rerun the bot by adding the props-bot label to the PR. Probably a good practice before manually reviewing and merging to be sure everyone is included.

@westonruter westonruter added Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature no milestone PRs that do not have a defined milestone for release labels Feb 2, 2024
github.event_name == 'pull_request_target' && github.event.action != 'labeled' ||
'props-bot' == github.event.label.name
) &&
( ! github.event.pull_request.draft && github.event.pull_request.state == 'open' || ! github.event.issue.draft && github.event.issue.state == 'open' )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the &&s are evaluated before the || in this line? Consider adding parenthesis to clarify the comparisons.

Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left one small question, not a blocker.

@westonruter westonruter merged commit 6b3494c into trunk Feb 2, 2024
14 of 19 checks passed
@westonruter westonruter deleted the improve/props-bot branch February 2, 2024 19:54
@westonruter
Copy link
Member

LOL. And naturally I didn't add the Co-Authored-By trailers to the merge commit message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants