-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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 broken WooCommerce Beta Tester release action #47593
Conversation
…it is part of build:zip command
Hi @ilyasfoo, @chihsuan, @ObliviousHarmony, @woocommerce/ghidorah, @woocommerce/vortex Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Hi @ilyasfoo, @chihsuan, @ObliviousHarmony, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested well! 👍 just a comment to confirm the build command is needed or not.
@@ -20,7 +20,8 @@ jobs: | |||
- name: Setup WooCommerce Monorepo | |||
uses: ./.github/actions/setup-woocommerce-monorepo | |||
with: | |||
install: '@woocommerce/plugin-woocommerce-beta-tester' | |||
install: '@woocommerce/plugin-woocommerce' | |||
build: '@woocommerce/plugin-woocommerce' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to build the woocommerce plugin for WooCommerce Beta Tester release? If not, I think we can remove the build command. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chihsuan To be honest, I don't fully understand why we need it...but it was needed to fix the errors. My best guess is that the packages in packages/js
are built with plugin-woocommerce
and that is needed in wc beta tester
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, sounds like improperly declared dependencies pnpm should handle that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@psealock I don't see anything obvious in https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce-beta-tester/package.json
What do you think about merging it for now and following up to look into it further?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because plugins/woocommerce-beta-tester
doesn't adhere to the monorepo's command structure. It only builds itself as opposed to all child dependencies. It needs to be updated with the correct build
, lint
, and watch:build
scripts along with their associated tool/language specific scripts.
In the interest of fixing the action I think it would be fine to move forward like this. We should, however, get the scripts updated in the beta tester. cc @woocommerce/vortex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ObliviousHarmony for the explanation. I'll go ahead and merge this for now.
Install and build the core -- removed woocomemrce beta install since it is part of build:zip command
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR fixes broken WooCommerce Beta Tester release action.
Changes:
plugin-woocommerce
install: @woocommerce/plugin-woocommerce-beta-tester
as install and build commands are part ofBuild WooCommerce Beta Tester Zip
workflowHow to test the changes in this Pull Request:
Run workflow
fix/fix-broken-beta-tester-release-action
.2.3.1
and run the job.fix/fix-broken-beta-tester-release-action
branch has the same changes, withoutCreate release
workflow for testing purposes.Changelog entry
Significance
Type
Message
Fix broken WooCommerce Beta Tester release action.
Comment