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

upkeep/110: modernize developer experience #166

Merged
merged 13 commits into from
Aug 2, 2024
Merged

upkeep/110: modernize developer experience #166

merged 13 commits into from
Aug 2, 2024

Conversation

Sidsector9
Copy link
Member

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

  • Node is updated to v20.
  • NPM is updated to v10.
  • Node and NPM changes are reflected in the Github actions workflows.
  • Build assets are generated by wordpress/scripts only.
  • Entry point for all assets are moved under src/

Closes #110

Steps to test the changes in this Pull Request:

  1. Pull the branch.
  2. Run nvm use or nvm install depending on the situation.
  3. Verify node and NPM versions by running node -v && npm -v.
  4. rm -rf node_modules/.
  5. npm ci && npm run build.
  6. Test the plugin functionality.
  7. Look for any 404 errors in console.
  8. Look for any errors in the debug.log file.
  9. Test the final ZIP.

Changelog entry

Dev - Update NPM packages and node version to v20 to modernize developer experience.

Copy link
Contributor

@dkotter dkotter left a comment

Choose a reason for hiding this comment

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

Left a handful of comments but in testing, things seemed to work great. I see we're changing the textdomain in lots of JS files from woocommerce-square to just woocommerce. I left a handful of comments on this but then decided to quit flagging it. If this isn't intentional, we should change those back. If there is a reason why, would be curious to know.

Also, E2E tests are failing on this PR. Not sure it's related but would be great to get those fixed up so we can be confident these updates aren't causing problems.

src/blocks/cash-app-pay/index.js Outdated Show resolved Hide resolved
src/new-user-experience/modules/advanced-settings/index.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@Sidsector9
Copy link
Member Author

@dkotter the tests are passing locally in the first pass:

Screenshot 2024-07-04 at 11 58 58 AM

I'll investigate why it is failing in pipeline.

@Sidsector9 Sidsector9 requested a review from dkotter July 15, 2024 11:15
@qasumitbagthariya
Copy link
Contributor

Regression / Smoke Test Report ✅

Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.5.5, npm version 8.19.4, node version 16.20.0)

Status- Working expected with Plugin Archive/Zip file same as fix specific branch.

Next Step- Ready to Merge 🚀

@vikrampm1 vikrampm1 removed this from the Future Release milestone Aug 2, 2024
@vikrampm1 vikrampm1 added this to the 4.7.3 milestone Aug 2, 2024
@vikrampm1 vikrampm1 marked this pull request as ready for review August 2, 2024 14:50
@vikrampm1 vikrampm1 mentioned this pull request Aug 2, 2024
18 tasks
@dkotter dkotter merged commit 48978f1 into trunk Aug 2, 2024
5 of 6 checks passed
@dkotter dkotter deleted the upkeep/110 branch August 2, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modernize developer experience (npm, Node, Composer, Grunt, wp/scripts)
5 participants