search

withastro / action

A GitHub Action that deploys your Astro project to GitHub Pages
Other
153 stars 34 forks source link

Moved away from set-output #15

Closed mandar1jn closed 1 year ago

mandar1jn commented 1 year ago

This changes the action to use environment variables instead of the set-output command because it’s being deprecated C60E8405-06B9-4BB3-BD02-18CFFB7CA11F The warning is still being thrown but that’s because of the external setup-node action. The warning thrown by this action are gone now

mandar1jn commented 1 year ago

From what I can see, I think PACKAGE_MANAGER and node_pm are interchangeable so one could theoretically be removed. I haven’t tested this though

swift502 commented 1 year ago

I just did a bunch of tests and I believe we can safely remove the step output and just use an environment variable.

The reason to prefer $GITHUB_ENV over $GITHUB_OUTPUT is because the overall code comes out more compact, and also we get automatically defined env variables for bash scripts.

As a result of that, these two lines also don't seem to be necessary:

https://github.com/withastro/action/blob/b221a41a8f32b5232393966f49d3a2e90001cdc7/action.yml#L68-L69

I tested it without them and it seems to work, the env variable is still defined. In fact, the "Install" step above already uses the implicitly defined variable.

Here's my testing version: https://github.com/swift502/GithubActionSandbox/blob/main/.github/workflows/deploy.yml And the successful job run: https://github.com/swift502/GithubActionSandbox/actions/runs/3401882888/jobs/5657289360

Unless I missed something (let me know), I can make a PR with my version of the script.

swift502 commented 1 year ago

@natemoo-re 👋 Any chance of getting this merged? I'd like to make a cleanup PR but it will probably have conflicts with this PR if I do it now.