-
Notifications
You must be signed in to change notification settings - Fork 95
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 DRY RUN
mode support that can be used while testing
#122
Add DRY RUN
mode support that can be used while testing
#122
Conversation
deploy.sh
Outdated
echo "➤ Committing files..." | ||
svn commit -m "Update to version $VERSION from GitHub" --no-auth-cache --non-interactive --username "$SVN_USERNAME" --password "$SVN_PASSWORD" | ||
else | ||
echo "➤ Debug mode: Files not committed." |
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 think this is the point where @joemcgill is looking for some sort of output to view what would have been sent across to SVN
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.
@jeffpaul For the unbundling of the Performance Lab plugin, we did a similar thing in which we downloaded the deploy.sh
file in our repo and removed this line of code so it would not commit to the SVN repo. Our main intention is that we can check all the processes that the deploy script performs but not commit the files to the SVN repository.
Check WordPress/performance#700 (comment) for more details.
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 @mukeshpanchal27 ! I added a few thoughts but more importantly, this does not appear to work the way it is intended to.
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 @mukeshpanchal27 – this is looking better, just a few more thoughts for you 👍
README.md
Outdated
@@ -23,6 +23,7 @@ This Action commits the contents of your Git tag to the WordPress.org plugin rep | |||
* `VERSION` - defaults to the tag name; do not recommend setting this except for testing purposes. | |||
* `ASSETS_DIR` - defaults to `.wordpress-org`, customizable for other locations of WordPress.org plugin repository-specific assets that belong in the top-level `assets` directory (the one on the same level as `trunk`). | |||
* `BUILD_DIR` - defaults to `false`. Set this flag to the directory where you build your plugins files into, then the action will copy and deploy files from that directory. Both absolute and relative paths are supported. The relative path if provided will be concatenated with the repository root directory. All files and folders in the build directory will be deployed, `.disignore` or `.gitattributes` will be ignored. | |||
* `DRY_RUN` - defaults to `false`. Set `true` to skip the final commit step and check all the debugging processes. |
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.
Looking at this again, I think this would actually make more sense as a proper input for the action, like generate-zip
. This makes it part of the actual API of the action and less fragile to variance of however someone might use/pass an environment variable. Ultimately it would still be an environment variable in the script, but one that isn't intended to be passed manually.
Then we could follow the same simple conditional style that is used for $INPUT_GENERATE_ZIP
action-wordpress-plugin-deploy/deploy.sh
Line 163 in f45e3de
if $INPUT_GENERATE_ZIP; then |
What do you think?
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 @aaemnnosttv, for the feedback. Do we need to add dry-run.yml
to this PR so others can see how it will be used?
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 think we would need a new dry-run.yml
file. I would suggest updating the action.yml file to set a default for the input
inputs:
generate-zip:
description: 'Generate package zip file?'
default: false
dry-run:
description: 'Run the deployment process without committing.'
default: false
And then set the environment variable based on the input here
env:
INPUT_GENERATE_ZIP: ${{ inputs.generate-zip }}
INPUT_DRY_RUN: ${{ inputs.dry-run }}
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 @joemcgill 👍
@mukeshpanchal27 this looks like the only part that's left to do here 🙂
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.
@mukeshpanchal27 This looks great, it's going to be quite handy for testing our debug workflows using this action!
Nothing to add to @aaemnnosttv's feedback from my end.
DEBUG
mode support that can be used while testingDRY RUN
mode support that can be used while testing
Description of the Change
Closes #121
Credits
Props @mukeshpanchal27 @joemcgill @felixarntz
Checklist: