Skip to content

Conversation

@dkoo
Copy link
Contributor

@dkoo dkoo commented Oct 30, 2025

All Submissions:

Changes proposed in this Pull Request:

See Automattic/newspack-scripts#219.

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 successfully ran tests with your changes locally?

@chickenn00dle
Copy link
Contributor

chickenn00dle commented Oct 30, 2025

  • Run npm start or npm ci (no --legacy-peer-deps!)
  • Confirm that the install completes successfully
  • Run npm run build and npm run watch and confirm that the assets build successfully
  • Run npm run lint and confirm that SCSS and JS linting works
  • Run npm run test and confirm that JS unit tests work (only applicable if the repo has any)
  • Run npm run semantic-release --dry-run and confirm there's successful output
  • Smoke test both WP admin and front-end functionality—there should be no significant changes

Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Running into an error at this step:

Run npm run semantic-release --dry-run and confirm there's successful output

 ✘  An error occurred while running semantic-release: Error: Cannot find module 'node-fetch'

@dkoo
Copy link
Contributor Author

dkoo commented Oct 30, 2025

@chickenn00dle ddab6ee should fix this by removing the node-fetch polyfill requirement (seems like it's no longer needed for Node v18+). Please run npm ci again before re-testing.

@dkoo dkoo requested a review from chickenn00dle October 30, 2025 19:56
@dkoo
Copy link
Contributor Author

dkoo commented Oct 30, 2025

Looks like that latest change to package-lock.json broke something else. Stand by...

@dkoo
Copy link
Contributor Author

dkoo commented Oct 30, 2025

@chickenn00dle Ok, we should be good now with bda4cb4. Looks like that ajv version override is no longer needed.

Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Thanks @dkoo! Working now 👍

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Changes or Feedback labels Oct 30, 2025
@dkoo dkoo marked this pull request as ready for review November 3, 2025 18:21
@dkoo dkoo requested a review from a team as a code owner November 3, 2025 18:21
@dkoo dkoo merged commit 98d01d0 into trunk Nov 3, 2025
3 checks passed
@dkoo dkoo deleted the chore/update-dependencies-oct-2025 branch November 3, 2025 18:21
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Hey @dkoo, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.15.1-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.15.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants