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 new github action output format #139

Merged
merged 1 commit into from
Oct 28, 2022
Merged

Use new github action output format #139

merged 1 commit into from
Oct 28, 2022

Conversation

p3lim
Copy link
Collaborator

@p3lim p3lim commented Oct 24, 2022

GitHub has deprecated the set-output meta-methods of actions/workflows for something better. The workflow throws a warning with the current implementation.

To read this output use ${{ steps.packager.outputs.archive_path }} (same as before).

More info: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
New docs: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-output-parameter

@p3lim p3lim requested a review from nebularg October 24, 2022 22:58
@p3lim
Copy link
Collaborator Author

p3lim commented Oct 24, 2022

Should probably hold on to this PR until 2023 as it's not compatible with old runners, they are removing the old way 31st May 2023.

@nebularg
Copy link
Member

nebularg commented Oct 25, 2022

Should probably hold on to this PR until 2023 as it's not compatible with old runners, they are removing the old way 31st May 2023.

I don't see where they said old runners don't support it? they've had this syntax for a while now, so old runners should support it, they just don't warn about stdout being depreciated obviously.

I added this and never documented it, so I doubt many people use it. I'd follow the GitHub convention with upper casing the env var and think it's fine to push it. Anyone that actually uses it will probably check here if it stopped working (i think the path in ${{}} is case insensitive, so it may not even affect anything) so it's probably worthwhile to get the notice out of everyone's log

release.sh Outdated Show resolved Hide resolved
@Molkree
Copy link

Molkree commented Oct 27, 2022

I'd follow the GitHub convention with upper casing the env var

i think the path in ${{}} is case insensitive, so it may not even affect anything

@nebularg

Fun fact: it's not env var, it's output var. And yes, there's a difference, output vars are NOT case sensitive while env vars ARE case sensitive.

My tests:

- name: Set color
  id: random-color-generator
  run: |
    echo "SELECTED_COLOR=green" >> $GITHUB_OUTPUT
    echo "selected_color=red" >> $GITHUB_OUTPUT
    echo "SELECTED_COLOR=green" >> $GITHUB_ENV
    echo "selected_color=red" >> $GITHUB_ENV
- name: Get colors
  run: |
    echo "The selected color is ${{ steps.random-color-generator.outputs.SELECTED_COLOR }}"
    echo "The selected color is ${{ steps.random-color-generator.outputs.selected_color }}"
    echo "The selected color is ${{ env.SELECTED_COLOR }}"
    echo "The selected color is ${{ env.selected_color }}"

image

Muehe added a commit to Questie/Questie that referenced this pull request Oct 27, 2022
Muehe added a commit to Questie/Questie that referenced this pull request Oct 27, 2022
@Muehe
Copy link

Muehe commented Oct 27, 2022

I'm in the process of adding this script to Questie so I had a chance to test this PR.

Log without PR (Questie/Questie@764f08d):

Generating changelog of commits into CHANGELOG.md
Warning: The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
Creating archive: Questie-v7.4.2-96-g764f08d.zip (v7.4.2-96-g764f08d)

Packaging complete.

Log with PR (Questie/Questie@f462809):

Generating changelog of commits into CHANGELOG.md
Creating archive: Questie-v7.4.2-97-gf462809.zip (v7.4.2-97-gf462809)

Packaging complete.

Note that I am using -d option, so no upload is actually happing and I'm not sure the zip is successfully created, but seems like it at least. Will get back to you when I have added API keys and such.

In conclusion, PR seems fine as is, without upper-casing.

@Molkree
Copy link

Molkree commented Oct 28, 2022

@nebularg, can you update the v2 tag? Thank you.

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.

4 participants