fix(action): Use environment variables for complex inputs - #716
Conversation
Refactor 'Set git user' and 'Request publish' steps in action.yml to use environment variables instead of direct string interpolation. This prevents syntax errors caused by unescaped special characters in fields like the changelog.
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Docker
Other
🤖 This preview updates automatically when you update the PR. |
| INPUT_GIT_USER_NAME: ${{ inputs.git_user_name }} | ||
| INPUT_GIT_USER_EMAIL: ${{ inputs.git_user_email }} |
There was a problem hiding this comment.
Why not doing this
| INPUT_GIT_USER_NAME: ${{ inputs.git_user_name }} | |
| INPUT_GIT_USER_EMAIL: ${{ inputs.git_user_email }} | |
| GIT_USER_NAME: ${{ inputs.git_user_name }} | |
| GIT_USER_EMAIL: ${{ inputs.git_user_email }} |
And then getting rid of the assigments on lines 84 and 85?
There was a problem hiding this comment.
Pretty sure we can also fold this logic into the env block, making it safer and tidier? The less bash the better?
| echo "GIT_COMMITTER_NAME=${GIT_USER_NAME}" >> $GITHUB_ENV | ||
| echo "GIT_AUTHOR_NAME=${GIT_USER_NAME}" >> $GITHUB_ENV | ||
| echo "EMAIL=${GIT_USER_EMAIL}" >> $GITHUB_ENV |
There was a problem hiding this comment.
I wonder if we can lift these up to job level env block and completly get rid of the Set git user step?
|
|
||
| # Use resolved version from Craft output | ||
| RESOLVED_VERSION="${{ steps.craft.outputs.version }}" | ||
| RESOLVED_VERSION="$VERSION" |
There was a problem hiding this comment.
Do we need the RESOLVED_VERSION - VERSION indirection? Why not map the value directly to RESOLVED_VERSION in the env block above?
| if [[ -n "$INPUT_MERGE_TARGET" ]]; then | ||
| merge_target="$INPUT_MERGE_TARGET" | ||
| else | ||
| merge_target='(default)' | ||
| fi |
There was a problem hiding this comment.
I think this logic can also be folded in to the env block above.
| # Use Craft outputs for git info | ||
| RELEASE_BRANCH="${{ steps.craft.outputs.branch }}" | ||
| RELEASE_SHA="${{ steps.craft.outputs.sha }}" | ||
| PREVIOUS_TAG="${{ steps.craft.outputs.previous_tag }}" | ||
| RELEASE_BRANCH="$BRANCH" | ||
| RELEASE_SHA="$SHA" | ||
| RELEASE_PREVIOUS_TAG="$PREVIOUS_TAG" | ||
|
|
||
| # Fall back to HEAD if no previous tag | ||
| if [[ -z "$PREVIOUS_TAG" ]]; then | ||
| PREVIOUS_TAG="HEAD" | ||
| if [[ -z "$RELEASE_PREVIOUS_TAG" ]]; then | ||
| RELEASE_PREVIOUS_TAG="HEAD" | ||
| fi |
There was a problem hiding this comment.
Same like others: this entire logic block can be put into the env block.
Address PR review comments by simplifying 'Set git user' and 'Request publish' steps. Move variable calculation and default value logic from bash into GitHub Actions expressions within 'env' blocks.
|
I have addressed the review comments by refactoring the bash logic into GitHub Actions expressions in the |
Description
This PR refactors the 'Set git user' and 'Request publish' steps in
action.ymlto use environment variables for potentially complex data instead of direct string interpolation in bash scripts.Problem
Injecting content like
${{ steps.craft.outputs.changelog }}directly into a bash script using single quotes fails if the content contains single quotes or other special characters (e.g.,syntax error near unexpected token '(').Solution
Pass these values as environment variables, which is the recommended and safe way to handle complex string data in GitHub Actions.
Changes
INPUT_GIT_USER_NAMEandINPUT_GIT_USER_EMAILenv vars.CHANGELOG,TARGETS,VERSION, etc.