-
Notifications
You must be signed in to change notification settings - Fork 312
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
Automatically update wp-cli.md #2370
Conversation
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.
Main question is if you really want to update the docs off develop or not.
.github/workflows/build-docs.yml
Outdated
- master | ||
pull_request: | ||
branches: | ||
- develop |
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.
We purposely do not update the docs off changes to develop as those items are not yet in a released version and thus the docs would conflict with whats actually in the plugin. You might consider removing this unless there's a precise reason to keep it?
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 only reason why I added that was to generate the artifact after creating the PR, I don't think there is any reason why we should update these docs when things are merged to develop. I'll push a commit shortly with what I think should be the "final" version of the action.
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.
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.
@felipeelia looks like that change dropped develop and the generating of the ZIP artifact yeah?
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.
Yup @jeffpaul.
- Remove executions on PRs - Remove artifact creation - Uncomment deployment to GH Pages
- name: Set PHP version | ||
uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: '7.4' | ||
coverage: none | ||
tools: prestissimo, composer:v1 | ||
ini-values: memory_limit=3G |
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 don't see the usage of composer
in this workflow, and PHP 7.4 is shipped with ubuntu-latest
. I'm wondering if we can remove this section?
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.
Good points @dinhtungdu!
- Composer is used by the wp package install command: https://github.com/wp-cli/package-command
- GH Actions have both PHP 7.4 and 8.0 available at this point. I think it will already use 8.0 by default but even if it is still using 7.4 it is just a matter of time to have 8.0 as the default. I'd rather force 7.4 until we are all sure EP is fully compatible with that version than risking it breaking. Also, we need that
memory_limit=3G
anyway because the package install command is a huge memory consumer.
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, then this is looking good to me! Note that I assume the cli-command-docs
works as expected.
Description of the Change
Based on the work made for #2369, this PR is an attempt to automate the process.