GitHub Actions: Fix PHP file change detection filter pattern #61183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit. Thank you! ❤️ |
t-hamano
added
[Type] Build Tooling
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Co-authored-by: Brian Alexander <[email protected]>
|
Size Change: 0 B Total Size: 1.74 MB ℹ️ View Unchanged
|
|
@ironprogrammer Thanks for the review! I made changes to some files after surrounding the pattern with quotes, but the file changes are no longer detected.
After that, when I deleted the quote, the change was detected successfully.
When I checked the README of the actual action, I found the following statement. Therefore, quotes may not be necessary for this action. |
|
In response to @t-hamano:
Ah, interesting -- good to know! |
|
@t-hamano, what do you think about addressing the intent behind the directory filters that were originally introduced in 3c34c48? The workflow's intent is stated as, "Confirm if PHP changes require backporting to WordPress Core", but this could include other file types that should be reviewed and/or synced. E.g. something like this: files: |
lib/**
packages/**/*.php
phpunit/**This update would isolate changes in repo locations that are known to be sync points or otherwise affect Core, including PHP changes in packages, and non-PHP assets (e.g. images and fonts for unit tests, etc). (I also discussed the possibility of a |
Yes, this pattern could be improved. I understand the background behind the introduction of this GitHub Actions, but it was updated to purely catch all PHP file changes (see this discussion) However, there are files other than PHP files that need to be backported to core, such as For example, how about targeting all PHP files and all files in the lib directory as shown below? |
|
Thanks for sharing that discussion. I guess it should be reconciled to clarify whether any PHP file change in the Gutenberg repo is intended here (including plugin-specific code in the root, The docs for Back-merging code to WordPress Core call out Additionally, I'd argue that PHP changes to |
ironprogrammer
approved these changes
May 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed regression caused by #59653
What?
I've noticed that PRs that include changes to PHP files don't automatically have a comment added to encourage them to backport to the core. This PR ensures that GitHub Actions correctly detects PHP changes and that the GitHub Actions adds a comment.
Below are examples of PRs showing that this Props Bot was not working.
textAlignblock support #59531Why?
In #59653, We changed the filter pattern as follows.
According to the documentation, curly brackets cannot be allowed as a pattern. In other words, it seems that the pattern
.{php}did not originally work. As a result, it seems that file changes are no longer detected at all.How?
Removed curly brackets. This should now detect changes to all PHP files.
Testing Instructions
By e81c37a I temporarily made changes to a PHP file. GitHub Actions correctly detected file changes and added a comment. Then I removed the PHP changes by e1b5219. GitHub Actions updated the existing comment with a comment indicating that no PHP files were changed in the latest commit.