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

add/lys xstate scaffolding #45548

Merged
merged 5 commits into from Mar 18, 2024
Merged

add/lys xstate scaffolding #45548

merged 5 commits into from Mar 18, 2024

Conversation

rjchow
Copy link
Contributor

@rjchow rjchow commented Mar 13, 2024

  • dev: added xstate 5
  • add xstate scaffolding for launch your store

Submission Review Guidelines:

Changes proposed in this Pull Request:

Closes #45116.

Note that this PR has installed xstate v5 simultaneously alongside xstate v4. There is a postinstall script which correctly symlinks the expected packages.

This arrangement will be required to be in place until we migrate the previous xstate machines over to v5.

How to test the changes in this Pull Request:

Please exclude from Solaris and GlobalStep testing as this is gated behind a feature flag and is part of the Launch Your Store feature which will be tested as a complete feature when the call for testing is released.

  1. Ensure that the "launch-your-store" feature flag is enabled using WC Beta Tester plugin
  2. Go to http://localhost:8888/wp-admin/admin.php?page=wc-admin&path=%2Flaunch-your-store and test around
Screen.Recording.2024-03-13.at.23.53.23.mov

Note that the browser back button will not work as expected as it has not been implemented and will be implemented in a follow up PR.

The XState inspector will also not work as the currently installed version only works for xstate v4 while this PR uses xstate v5. This will also be addressed in a follow up PR. In the mean time please use the debug output in this PR to assist.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@github-actions github-actions bot added focus: monorepo infrastructure Issues and PRs related to monorepo tooling. plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Mar 13, 2024
@rjchow rjchow marked this pull request as ready for review March 13, 2024 15:55
@rjchow rjchow requested review from a team, ilyasfoo and adrianduffell March 13, 2024 15:55
@rjchow rjchow force-pushed the add/lys-xstate-scaffolding branch from b560ed7 to de9f3de Compare March 13, 2024 16:00
Copy link
Contributor

Hi @ilyasfoo, @adrianduffell, @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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

Copy link
Contributor

github-actions bot commented Mar 13, 2024

Test Results Summary

Commit SHA: 13f4245

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 38s
E2E Tests342001103537m 39s

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.

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

Awesome, testing well! 👍

I just have one feedback. I found it a bit difficult to know whether the file is for the sidebar, main content page, or xstate machine definition from the filenames and paths. It's not a problem now, but I think it would be better if we could tell at a glance. 🙂

Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

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

It tested well! Thanks @rjchow 🚀

@chihsuan's suggestion and the inspector support for v5 sound great as follow-up issues. If you agree, could you please create those 🙏

@rjchow rjchow force-pushed the add/lys-xstate-scaffolding branch from de9f3de to 13f4245 Compare March 18, 2024 06:31
@rjchow
Copy link
Contributor Author

rjchow commented Mar 18, 2024

Thanks for the reviews @adrianduffell and @chihsuan! I'm doing the folder organisation this PR since it's a minor work to do here and a fair bit of work to put a new PR up for.

@rjchow rjchow merged commit dbd577c into trunk Mar 18, 2024
91 checks passed
@rjchow rjchow deleted the add/lys-xstate-scaffolding branch March 18, 2024 07:44
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 18, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 18, 2024
@alvarothomas alvarothomas added status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 19, 2024
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. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Launch Your Store] add xstate scaffolding
4 participants