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 deprecation warning for set-output #2370

Closed
wants to merge 4 commits into from
Closed

Fix deprecation warning for set-output #2370

wants to merge 4 commits into from

Conversation

nathanchapman
Copy link
Sponsor Contributor

@nathanchapman nathanchapman commented Mar 6, 2023

🚨 IMPORTANT: Please do not create a Pull Request without creating an issue first.

Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of
the pull request.

Description

Addresses second half of GitHub Actions Deprecation notices re:

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/

See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/#patching-your-actions-and-workflows

This was half-fixed by a renovate update PR (#2301), which upgraded @actions/core to 1.10.0. Now, action/index.js just needs to be rebuilt and released.

Fixes #2196

The other deprecation notice is addressed by #2221 (upgrade from Node12 to Node16)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take
action on it as appropriate

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Environment:

  • OS:
  • @graphql-inspector/...:
  • NodeJS:

Checklist:

  • I have followed the
    CONTRIBUTING doc and the
    style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose
the solution you did and what alternatives you considered, etc...

@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2023

⚠️ No Changeset found

Latest commit: 1d5e659

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dimaMachina
Copy link
Collaborator

plz rebase, also deprecated set output was refactored in this PR #2358

@nathanchapman
Copy link
Sponsor Contributor Author

@B2o5T conflicts resolved. Note that #2358 only fixes deprecation warnings within https://github.com/kamilkisiela/graphql-inspector, not for all of the repos that use the graphql-inspector GitHub Action. Those should be resolved by cutting a release.

@nathanchapman
Copy link
Sponsor Contributor Author

@B2o5T resolved conflicts again. Should I remove the changes to action/index.js? does that get automatically regenerated on release?

@nathanchapman
Copy link
Sponsor Contributor Author

Confirmed the 3.4.11 release addresses the issue with set-ouput. I'll open a separate PR for the typo fixes here

@TuvalSimha
Copy link
Collaborator

@nathanchapman This is still relevant?

@nathanchapman
Copy link
Sponsor Contributor Author

nathanchapman commented Apr 18, 2023

@TuvalSimha

Not anymore, the necessary change (regenerating action/index.js) made it into master, but just needed to be released. 3.4.11 did the trick. I moved the other typo fixes in this PR into #2445

As for the issue #2196, that won't be fully solved until another release is created containing the changes from #2221

@nathanchapman
Copy link
Sponsor Contributor Author

Just noticed that another release was created ~6 hours ago, going to try to verify the fix with that new version

@nathanchapman
Copy link
Sponsor Contributor Author

Confirmed the fix, thanks @TuvalSimha!

@TuvalSimha
Copy link
Collaborator

@nathanchapman Great news! 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.

Github Action Deprecation notices
3 participants