-
Notifications
You must be signed in to change notification settings - Fork 108
Description
Currently the SVN credentials are required for all runs, however they're only used when performing the commit which is only conditionally called if the job is not running as a dry-run.
action-wordpress-plugin-deploy/deploy.sh
Lines 13 to 21 in 9273e6d
| if [[ -z "$SVN_USERNAME" ]]; then | |
| echo "Set the SVN_USERNAME secret" | |
| exit 1 | |
| fi | |
| if [[ -z "$SVN_PASSWORD" ]]; then | |
| echo "Set the SVN_PASSWORD secret" | |
| exit 1 | |
| fi |
action-wordpress-plugin-deploy/deploy.sh
Lines 193 to 198 in 9273e6d
| if $INPUT_DRY_RUN; then | |
| echo "➤ Dry run: Files not committed." | |
| else | |
| echo "➤ Committing files..." | |
| svn commit -m "Update to version $VERSION from GitHub" --no-auth-cache --non-interactive --username "$SVN_USERNAME" --password "$SVN_PASSWORD" | |
| fi |
The dry-run was a much more recent addition, so it's possible this wasn't considered at the time.
Describe the solution you'd like
Since the credentials are only needed for that part of the deploy which isn't called for a dry-run, it would be nice to not require them but only in this scenario. Instead, I propose that missing credentials only raise a warning in a dry-run (to ensure devs are still aware without failing the script) but are still required for normal runs. This would add a little peace of mind to dry-runs that a deployment could not be possible (even if there was a bug in the action's script).
Similarly, it could be worth even redefining/unsetting the SVN credential variables when running as a dry-run early on to further safeguard these runs when live credentials may be present.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status