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.

Sign up for GitHub

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

Jump to bottom

Add script for running pnpm commands in Jetpack repo #5667

Merged
merged 2 commits into from
Apr 24, 2023
Merged

Add script for running pnpm commands in Jetpack repo #5667

merged 2 commits into from
Apr 24, 2023

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Apr 13, 2023 •
edited

The idea behind adding this script is to allow running any command in the Jetpack repo. Currently, we only run install command to install the dependencies. In the next PR, to solve the issue mentioned here, we'll run the script to build the VideoPress package.

To test:

  1. Run npm i.
  2. Observe that the command finish successfully.
  3. Observe that the node version is the same before and after running the command.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot self-assigned this Apr 13, 2023
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Apr 13, 2023 •
edited

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@fluiddot fluiddot marked this pull request as ready for review April 13, 2023 18:16
@fluiddot fluiddot requested review from jhnstn and geriux April 13, 2023 18:16
@fluiddot fluiddot mentioned this pull request Apr 13, 2023
Merged
2 tasks
@fluiddot
Copy link
Contributor Author

Setting the PR back to draft because I'm getting the following error when trying the build a Jetpack plugin with the new script.

 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "./gutenberg-mobile/jetpack/projects/plugins/jetpack".

Expected version: ^18.13.0
Got: 14.21.2

@fluiddot fluiddot marked this pull request as draft April 13, 2023 18:36
jhnstn
jhnstn approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

@jhnstn jhnstn left a comment

Choose a reason for hiding this comment

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

Looks good!

@fluiddot
Copy link
Contributor Author

Setting the PR back to draft because I'm getting the following error when trying the build a Jetpack plugin with the new script.

 ERR_PNPM_UNSUPPORTED_ENGINE  Unsupported environment (bad pnpm and/or Node.js version)

Your Node version is incompatible with "./gutenberg-mobile/jetpack/projects/plugins/jetpack".

Expected version: ^18.13.0
Got: 14.21.2

The fix provided in #5689 should address this issue 🎊 .

@fluiddot fluiddot added the [Type] Enhancement Improves a current area of the editor label Apr 21, 2023
fluiddot
fluiddot commented Apr 21, 2023
View reviewed changes
Copy link
Contributor Author

@fluiddot fluiddot left a comment •
edited

Choose a reason for hiding this comment

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

The PR is ready to review again 🎊 .

bin/run-jetpack-command.sh
Comment on lines -35 to -36
# Install dependecies of Jetpack plugin
pushd projects/plugins/jetpack
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the future incorporation of the VideoPress package, seems more practical to install the dependencies from the root level.

bin/run-jetpack-command.sh
Comment on lines +44 to +45
# Revert to Gutenberg node version
nvm use
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed when running this script that after the execution, I was on a different node version. Hence, I added that nvm changes the node version to the one recommended in Gutenberg/Gutenberg Mobile.

@fluiddot fluiddot marked this pull request as ready for review April 21, 2023 10:46
@fluiddot fluiddot requested a review from jhnstn April 21, 2023 10:46
geriux
geriux approved these changes Apr 24, 2023
View reviewed changes
Copy link
Member

@geriux geriux left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work! I was able to run npm i successfully and I was also able to generate the bundles and strings correctly 🚀

@fluiddot fluiddot merged commit e0c3aea into trunk Apr 24, 2023
@fluiddot fluiddot deleted the add-run-jetpack-command branch April 24, 2023 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geriux geriux geriux approved these changes

@jhnstn jhnstn Awaiting requested review from jhnstn

Assignees

@fluiddot fluiddot

Labels
Tooling [Type] Enhancement Improves a current area of the editor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fluiddot @jhnstn @geriux