-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add action for Hook Documentation Generation #2060
Conversation
At first, this Workflow was triggered when a push event happened with some PHP files. After discussion with the team, now we changed the trigger to only run on release branches. You can see the trigger in another release branch here: #2064 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes they seem to be working well as far as I can see. The workflow is triggered and commits a new version of src/Hooks/README.md
when a release branch is update.
As far as I understand we also want to maintain only one script within our grow repo. So the script bin/HooksDocsGenerator.php
can be removed.
I just left a few minor comments but this should be pretty much good to go, so I'll mark the PR as approved.
@@ -0,0 +1,57 @@ | |||
name: PHP Hook Documentation Generator | |||
|
|||
on: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The push and pull_request (with the PHP filter), look fine to me. But I was wondering if it would do any harm to also allow us to run this manually, like here. It's not going to effect our automated flows, and will make it easier if we ever want to run it manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented this suggestions
uses: woocommerce/grow/hook-documentation@actions-v1 | ||
with: | ||
debug-output: yes | ||
source-directories: src/,views/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the main plugin file google-listings-and-ads.php
? Is this strictly a list of directories or does it support single files as well?
It so happens we don't have any hooks there now, but considering we want to use it for other extensions the script should be flexible enough to include the main plugin file. If that's not possible then can we log an issue in the Grow repo that this should be fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like its possible as internally it uses Finder()->path()
You can see it here in a test when I added gla_test_action
inside the main plugin file.
https://github.com/woocommerce/google-listings-and-ads/actions/runs/5902917550/job/16011805311
Changes proposed in this Pull Request:
This PR creates an Action for generating PHP Hooks documentation when PHP files change.
It will commit a README.md in src/Hooks/README.md
Screenshots
Detailed test instructions:
Additional details:
Changelog entry