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

fix: update deprecated commands #65

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

mrdoodles
Copy link
Contributor

@mrdoodles mrdoodles commented Oct 23, 2022

Fix issue #63

updated

Update deprecated set-output to use the new format for multi-line strings as described in the GitHub documentation:

(https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings)

Also updated actions/checkout to the non-deprecated v3.

@austinpray-mixpanel Needed to update this for multi-line string handling, can you review please, thanks

Comment on lines +48 to +50
echo "results<<EOF" >> $GITHUB_OUTPUT
echo "${RESULTS}" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT

Choose a reason for hiding this comment

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

wait does this actually work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just a simple way e.g.
echo "results=${RESULTS}" >> $GITHUB_OUTPUT ?
EOF syntax makes sense to use for multiline things, but there is one line echo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@mrdoodles mrdoodles Nov 11, 2022

Choose a reason for hiding this comment

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

@ViacheslavKudinov Because during my test the output actually was a multiline string so it failed with the simple format which was my original commit that failed and i force pushed over.

You can see the output from using the simple method here: https://github.com/hadolint/hadolint-action/actions/runs/3307229271/jobs/5671320194

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Thanks @mrdoodles for clarification.

Copy link
Contributor Author

@mrdoodles mrdoodles Nov 11, 2022

Choose a reason for hiding this comment

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

Having said that I have just tried it again and it has passed:

My local test pull: https://github.com/mrdoodles/hadolint-local/pull/7
The workflow run: https://github.com/mrdoodles/hadolint-local/actions/runs/3443878466/jobs/5745890302

Could it be that sometimes there is multiline output in which case the longer format offers a better solution as it handles both scenarios?

Copy link

@maxhelias maxhelias Dec 23, 2022

Choose a reason for hiding this comment

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

The delimeter shouldn't be randomly generated? Like in this article : https://lab.amalitsky.com/posts/2022/github-actions-set-output-migration/

@lorenzo
Copy link
Member

lorenzo commented Nov 11, 2022

thanks!

@lorenzo lorenzo merged commit 67d715b into hadolint:master Nov 11, 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.

5 participants