Skip to content
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

Use different environment variable for instance setup #6

Closed
wants to merge 19 commits into from

Conversation

subhojit-axl
Copy link

Fixes #5

@subhojit-axl
Copy link
Author

subhojit-axl commented Mar 16, 2022

@hussainweb @skippednote Can you please take a look. GITHUB_HEAD_REF is always present during the pull request creation and merge, and most importantly, the value stays the same.

@hussainweb
Copy link
Member

The problem is that GITHUB_HEAD_REF is only used when the job runs in a pull_request workflow run (see the docs). If we use this in the action, then the action may only be used in such jobs and we don't want that.

We should explore making this an input parameter. That way, the calling job can set the branch to whatever they want. By default, we could use GITHUB_REF_NAME.

@subhojit-axl
Copy link
Author

@hussainweb Agree. Makes sense. I will take a look.

action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@subhojit-axl
Copy link
Author

@hussainweb This can be reviewed now.

@@ -11,7 +11,7 @@ mkdir -p ~/.ssh && chmod 0700 ~/.ssh
cat ${GITHUB_ACTION_PATH}/known_hosts >> ~/.ssh/known_hosts

platform project:set-remote ${PLATFORM_PROJECT_ID}
PLATFORM_OPTS="-vv --activate --target ${GITHUB_REF_NAME}"
PLATFORM_OPTS="-vv --activate --target $ENVIRONMENT_NAME"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PLATFORM_OPTS="-vv --activate --target $ENVIRONMENT_NAME"
[[ "$ENVIRONMENT_NAME" == "" ]] && ENVIRONMENT_NAME="$GITHUB_REF_NAME"
PLATFORM_OPTS="-vv --activate --target $ENVIRONMENT_NAME"

Comment on lines +35 to +42
-
name: Set environment name
id: set_environment_name
shell: bash
run: |
ENVIRONMENT_NAME=${{ inputs.environment-name }}
[[ "$ENVIRONMENT_NAME" == "" ]] && ENVIRONMENT_NAME="$GITHUB_REF_NAME"
echo "::set-output name=url::$ENVIRONMENT_NAME"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very fond of this. I think we can handle this in the shell script itself quite robustly. See my other two suggestions.

- run: ${{ github.action_path }}/deploy.sh
shell: bash
env:
SSH_PRIVATE_KEY: ${{ inputs.ssh-private-key }}
PLATFORM_PROJECT_ID: ${{ inputs.project-id }}
PLATFORMSH_CLI_TOKEN: ${{ inputs.cli-token }}
FORCE_PUSH: ${{ inputs.force-push }}
ENVIRONMENT_NAME: ${{ steps.set_environment_name.outputs.url }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ENVIRONMENT_NAME: ${{ steps.set_environment_name.outputs.url }}
ENVIRONMENT_NAME: ${{ inputs.environment-name }}

Comment on lines +35 to +42
-
name: Set environment name
id: set_environment_name
shell: bash
run: |
ENVIRONMENT_NAME=${{ inputs.environment-name }}
[[ "$ENVIRONMENT_NAME" == "" ]] && ENVIRONMENT_NAME="$GITHUB_REF_NAME"
echo "::set-output name=url::$ENVIRONMENT_NAME"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-
name: Set environment name
id: set_environment_name
shell: bash
run: |
ENVIRONMENT_NAME=${{ inputs.environment-name }}
[[ "$ENVIRONMENT_NAME" == "" ]] && ENVIRONMENT_NAME="$GITHUB_REF_NAME"
echo "::set-output name=url::$ENVIRONMENT_NAME"

@hussainweb hussainweb closed this Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue while cleaning up the new instance
3 participants