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.

Sign up for GitHub

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

Jump to bottom

workflow: Remove if from env conditional #9644

Merged
merged 1 commit into from
May 16, 2024
Merged

workflow: Remove if from env conditional #9644

merged 1 commit into from
May 16, 2024

Conversation

stevenhorsman
Copy link
Member

@stevenhorsman stevenhorsman commented May 16, 2024 •
edited

  • It appears like the if isn't required when setting env as a conditional :( Sorry!

@stevenhorsman stevenhorsman added force-skip-ci Stop the CI running for a PR (remember: if you break it, you fix it!) ok-to-test labels May 16, 2024
@katacontainersbot katacontainersbot added the size/tiny Smallest and simplest task label May 16, 2024
fidencio
fidencio approved these changes May 16, 2024
View reviewed changes
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @stevenhorsman, and welcome to GHA whac-a-mole session! :-)

@stevenhorsman
workflow: Fix problems with build-asset workflows
f7fd2f9 
- It appears like the `if` isn't required when setting env as a
conditional
- `inputs.stage` over input.stage
- Swap matrix.component to matrix.asset

Signed-off-by: stevenhorsman <steven@uk.ibm.com>
@katacontainersbot katacontainersbot added size/small Small and simple task and removed size/tiny Smallest and simplest task labels May 16, 2024
@stevenhorsman
Copy link
Member Author

As this is fixing a broken workflow on main and we run the workflow on main in PRs, I'll need to use admin powers to push this PR, but will wait for two approvals first.

wainersm
wainersm reviewed May 16, 2024
View reviewed changes
.github/workflows/build-kata-static-tarball-amd64.yaml
@@ -96,10 +96,10 @@ jobs:
ARTEFACT_REGISTRY_USERNAME: ${{ github.actor }}
ARTEFACT_REGISTRY_PASSWORD: ${{ secrets.GITHUB_TOKEN }}
TARGET_BRANCH: ${{ inputs.target-branch }}
RELEASE: ${{ if inputs.stage == 'release' && 'yes' || 'no' }}
RELEASE: ${{ inputs.stage == 'release' && 'yes' || 'no' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @stevenhorsman !

Sorry, I reviewed this and completely overlooked all those mistakes! I should take some vacation...oh wait...

Seriously now, let's enable this https://github.com/marketplace/actions/actionlint action as part of the static-checks to lint the workflow files. Probably some on the mistakes here we would have caught.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried running actionlint locally and (probably due to user error) it didn't detect this issues. I think it's worth a try anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm Interesting

@stevenhorsman stevenhorsman merged commit 03c0858 into kata-containers:main May 16, 2024
208 of 262 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wainersm wainersm wainersm approved these changes

@fidencio fidencio fidencio approved these changes

Assignees
No one assigned
Labels
force-skip-ci Stop the CI running for a PR (remember: if you break it, you fix it!) ok-to-test size/small Small and simple task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stevenhorsman @fidencio @wainersm @katacontainersbot