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

Migrate uses of ::set-output in code-analyzer to setOutput. #35895

Merged
merged 2 commits into from
Dec 12, 2022

Conversation

samueljseay
Copy link
Contributor

@samueljseay samueljseay commented Dec 8, 2022

All Submissions:

Changes proposed in this Pull Request:

This is a final set of changes to migrate all uses of set-output to write to the GITHUB_OUTPUT file instead. To do that here we install @actions/core and call setOutput.

It should be noted that calling setOutput will still throw a set-output warning right now because that library actually writes to GITHUB_OUTPUT as well as calling set-output. See actions/toolkit#1218

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

I think the simplest way to test this is with some basic code modification of the calling functions that print out found changes.

  1. Fork WooCommerce, setup actions. See Remove some usages of set-output #35799 for example

  2. Don't merge this PR but instead, merge this slightly modified branch: https://github.com/woocommerce/woocommerce/tree/dev/test-ca-output This has the same setOutput code, but passes in some dummy change data.

  3. When the CI check runs on your fork, see that it outputs some dummy information for all the lint checks.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

Test Results Summary

Commit SHA: 4615456

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 2s
E2E Tests187006019313m 7s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@samueljseay samueljseay requested review from a team and psealock and removed request for a team December 8, 2022 08:52
Copy link
Contributor

@psealock psealock left a comment

Choose a reason for hiding this comment

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

This is a final set of changes to migrate all uses of set-output

I'm finding a few more instances of ::set-output in the codebase, otherwise these changes are looking good 🚢

@samueljseay
Copy link
Contributor Author

@psealock those will just be the ones that haven't been merged in my other open PRs 😄

@samueljseay samueljseay merged commit dedbf7b into trunk Dec 12, 2022
@samueljseay samueljseay deleted the dev/migrate-ca-output branch December 12, 2022 06:41
@github-actions github-actions bot added this to the 7.3.0 milestone Dec 12, 2022
samueljseay added a commit that referenced this pull request Dec 15, 2022
* Migrate uses of ::set-output to setOutput.
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.

None yet

2 participants