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

Get rid of set-output commands in the workflows #969

Merged
merged 2 commits into from
Oct 17, 2022

Conversation

jkremser
Copy link
Member

Fixes #965

as suggested here and here, the echo "::set-output name=foo::${bar}" should be replaced with echo foo=bar >> $GITHUB_ENV and then read as normal env var in all subsequent steps in the same job.

@jkremser jkremser force-pushed the issue-965-set-output branch from 0bfbc04 to 61c8023 Compare October 14, 2022 14:49
@kuritka
Copy link
Collaborator

kuritka commented Oct 14, 2022

Hi, can't this generate some kind of scope issues ? ENV exists once for whole workflow run while step.. points to property of concrete instance.

Anyway, using env certainly simplifies the usage.

@jkremser
Copy link
Member Author

jkremser commented Oct 14, 2022

@kuritka what do you mean by scope issues? I agree that the semantics is different here and previously it was better isolated in the step itself. I haven't tested the behavior, but I can imagine that it's appending key=value entries to some env file which is being sourced at the beginning of each subsequent step (it's not available for the step itself in which we do the thing).

So I think the latter occurrences will override the previous ones, but I didn't test it. Anyway we do not do that nor are relying on such behavior AFAICT. One can also set the env vars explicitly for whole job but also for each step, in this case I have no idea about the precedence rules, it's not well documented either. For sharing the data between the jobs this will not work at all, it's designed to work under one job and for that use-case one has to use job outputs.

kuritka
kuritka previously approved these changes Oct 14, 2022
Copy link
Collaborator

@kuritka kuritka left a comment

Choose a reason for hiding this comment

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

lgtm

@somaritane
Copy link
Contributor

@kuritka we're basically following GH standard recommendations here for deprecated output commands

somaritane
somaritane previously approved these changes Oct 15, 2022
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

lgtm

@kuritka
Copy link
Collaborator

kuritka commented Oct 15, 2022

@somaritane - thx I see. I read it in links @jkremser mentioned in description. I'm convinced that the change is right.

ytsarev
ytsarev previously approved these changes Oct 17, 2022
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

LGTM

@jkremser jkremser dismissed stale reviews from ytsarev, somaritane, and kuritka via 1a9c02c October 17, 2022 13:00
@jkremser
Copy link
Member Author

sorry, I forgot to change the way how verbose flag was read in the print-debug action so I am adding a new commit: 1a9c02c to fix it.

The rest is the same as before, the review should be easier this way, in the end I will squash the changes.

@jkremser jkremser merged commit f997433 into k8gb-io:master Oct 17, 2022
@jkremser jkremser deleted the issue-965-set-output branch October 17, 2022 15:04
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.

node@12 has been deprecated warnings
4 participants