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

Another set of migrations from set-output to GITHUB_OUTPUT #35843

Merged
merged 7 commits into from
Dec 15, 2022

Conversation

samueljseay
Copy link
Contributor

@samueljseay samueljseay commented Dec 6, 2022

All Submissions:

Changes proposed in this Pull Request:

set-output is deprecated and will be removed early next year. This includes another set of migrations from set-output such as in #35799

Also note that saving these files ran Prettier against them too so where the formatting was incorrect that has also been fixed.

Note: I'm introducing these in chunks to make the test effort easier and to limit the risk of introducing wide ranging changes all at once.

How to test the changes in this Pull Request:

  1. You will need a fork of WooCommerce with Actions enabled. See testing instructions in Remove some usages of set-output #35799 if you need help with this.

  2. Merge this branch into trunk of your fork

  3. Test the is-community script. Create an arbitrary PR into your fork under your account. This should not be marked as a community contribution, check the CI check worked correctly and there were no errors. To test the case for community contributors, ask a team mate to submit an arbitrary PR to your fork. It should be marked as community contribution as long as they don't have write access to your fork.

  4. In your fork, check what the stable version of WooCommerce is (in the readme.txt). Create a release in releases that is a version less than stable. Check under actions you should be able to see a "post release" action has been run. It should have a skipped step because the version released is less than current stable. Now create a new release that is greater than the stable release version, this workflow should run all steps and none should be skipped. (for both cases obviously no errors should be shown).

  5. For the cherry pick, I think its quite difficult to test. @jonathansadowski have you tested cherry pick in PRs before?

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

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 6, 2022

Test Results Summary

Commit SHA: 574f047

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 54s
E2E Tests187006019317m 34s

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 changed the title [WIP] Another set of migrations from set-output to GITHUB_OUTPUT Another set of migrations from set-output to GITHUB_OUTPUT Dec 6, 2022
// If the release version is less than stable version we can bail.
if ( version.localeCompare( stableVersion, undefined, { numeric: true, sensitivity: 'base' } ) == -1 ) {
console.log( 'Release version is less than stable version. No automated action taken. A manual process is required.' );
core.setOutput( 'continue', 'false' )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a heads up, you'll still see the deprecation warning when core.setOutput is run because even as of latest it still does a set-output command after doing the command to write to GITHUB_OUTPUT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@samueljseay samueljseay requested review from a team and roykho and removed request for a team December 8, 2022 00:13
@samueljseay samueljseay marked this pull request as ready for review December 8, 2022 00:14
@samueljseay samueljseay requested a review from a team December 8, 2022 00:14
@roykho
Copy link
Member

roykho commented Dec 8, 2022

Seeing an error on community-label.yml workflow.

Run node .github/workflows/scripts/is-community-contributor.js
node:internal/modules/cjs/loader:988
  throw err;
  ^

Error: Cannot find module '@action/core'
Require stack:
- /home/runner/work/woocommerce-fork/woocommerce-fork/.github/workflows/scripts/is-community-contributor.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:985:15)
    at Function.Module._load (node:internal/modules/cjs/loader:833:27)
    at Module.require (node:internal/modules/cjs/loader:1057:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at Object.<anonymous> (/home/runner/work/woocommerce-fork/woocommerce-fork/.github/workflows/scripts/is-community-contributor.js:3:14)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:8[6](https://github.com/roykho/woocommerce-fork/actions/runs/3651683553/jobs/6169190334#step:6:7)[8](https://github.com/roykho/woocommerce-fork/actions/runs/3651683553/jobs/6169190334#step:6:9):[12](https://github.com/roykho/woocommerce-fork/actions/runs/3651683553/jobs/6169190334#step:6:13))
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/runner/work/woocommerce-fork/woocommerce-fork/.github/workflows/scripts/is-community-contributor.js'
  ]
}

@samueljseay
Copy link
Contributor Author

Whoops! sorry for the typo @roykho should be good now.

@roykho
Copy link
Member

roykho commented Dec 8, 2022

Getting this error on this step "If community PR, assign a reviewer".

Run shufo/auto-assign-reviewer-by-files@f5f3db9ef06bd72ab6978996988c6462cbdaabf6
  with:
    config: .github/project-community-pr-assigner.yml
Error: Input required and not supplied: token

@samueljseay
Copy link
Contributor Author

samueljseay commented Dec 8, 2022

@roykho thats not related to this. Thats because that action uses a token set in our repo. Probably I missed this as a testing step but you could add a secret called PR_ASSIGN_TOKEN to your fork.. Maybe a PAT generated in your account. I'm not sure though if that will succeed I haven't tested it.

Probably it doesn't matter though, the fact that this action is run in your fork is proof enough for me anyway

Copy link
Member

@roykho roykho left a comment

Choose a reason for hiding this comment

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

I think fixing this last issue I found should take care of it.

.github/workflows/cherry-pick.yml Outdated Show resolved Hide resolved
Co-authored-by: Roy Ho <roykho77@gmail.com>
roykho
roykho previously approved these changes Dec 14, 2022
Copy link
Member

@roykho roykho left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot added focus: react admin package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/customer-effort-score issues related to @woocommerce/customer-effort-score package: @woocommerce/data issues related to @woocommerce/data package: @woocommerce/onboarding issues related to @woocommerce/onboarding plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Dec 15, 2022
@github-actions github-actions bot removed the package: @woocommerce/onboarding issues related to @woocommerce/onboarding label Dec 15, 2022
@github-actions github-actions bot removed focus: react admin package: @woocommerce/components issues related to @woocommerce/components plugin: woocommerce Issues related to the WooCommerce Core plugin. package: @woocommerce/customer-effort-score issues related to @woocommerce/customer-effort-score package: @woocommerce/data issues related to @woocommerce/data labels Dec 15, 2022
@samueljseay samueljseay added the focus: monorepo infrastructure Issues and PRs related to monorepo tooling. label Dec 15, 2022
@samueljseay samueljseay merged commit 03d52ff into trunk Dec 15, 2022
@samueljseay samueljseay deleted the dev/more-setoutput-migration branch December 15, 2022 22:41
@github-actions github-actions bot added this to the 7.4.0 milestone Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: monorepo infrastructure Issues and PRs related to monorepo tooling.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants