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

Bump lerna to v4 #2017

Merged
merged 2 commits into from
Aug 22, 2022
Merged

Bump lerna to v4 #2017

merged 2 commits into from
Aug 22, 2022

Conversation

tlylt
Copy link
Contributor

@tlylt tlylt commented Aug 18, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Bumping lerna to v4 since we have migrated to nodejs v14 and above. Should be a trivial update, see https://github.com/lerna/lerna/releases/tag/v4.0.0 for details

  • tried bumping it to v5 and it seems there's some warning for npm/nodejs, so moving it to v4 for now.
  • the font files are updated as well as they appear as uncommitted changes after running npm run updatetest

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Bump lerna to v4


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@ang-zeyu
Copy link
Contributor

@tlylt I get an extremely large diff as well (but it looks correct on some eyeballing) on package-lock (slightly less lines than yours), but no updates to the font files.

What command are you using? (I'm using just npx lerna bootstrap --hoist) The ci part is skipped since package-lock and package.json are out of sync once you bump lerna's version number.

@ang-zeyu
Copy link
Contributor

Put up a diff here

@tlylt
Copy link
Contributor Author

tlylt commented Aug 21, 2022

@tlylt I get an extremely large diff as well (but it looks correct on some eyeballing) on package-lock (slightly less lines than yours), but no updates to the font files.

What command are you using? (I'm using just npx lerna bootstrap --hoist) The ci part is skipped since package-lock and package.json are out of sync once you bump lerna's version number.

Hmm I changed the Lerna version number in the root package.json to v4, then run npm run setup again, after which if I run npm run updatetest the font file diffs show up.

@tlylt
Copy link
Contributor Author

tlylt commented Aug 21, 2022

npx lerna bootstrap --hoist

This is done before or after the version bump? If before it might happen that the lerna that you are invoking is still v3.22.1? Could that be the reason why there's a difference?

@ang-zeyu
Copy link
Contributor

Hmm I changed the Lerna version number in the root package.json to v4, then run npm run setup again, after which if I run npm run updatetest the font file diffs show up.

Hmm I'm surprised this would even work (it shouldn't). The npm ci part should throw you an error that your package.json and package-lock.json are out of sync.

npm ERR! `npm ci` can only install packages when your package.json and package-lock.json
or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.

This is done before or after the version bump? If before it might happen that the lerna that you are invoking is still v3.22.1? Could that be the reason why there's a difference?

Before the bump, use lerna itself to update lerna 🙃. Then re-verified with npm run setup that there are no further diffs.

@tlylt
Copy link
Contributor Author

tlylt commented Aug 21, 2022

Hmm I'm surprised this would even work (it shouldn't). The npm ci part should throw you an error that your package.json and package-lock.json are out of sync.

I see, I guess I did 'npm install --save-dev lerna@v4.0.0' first, then run 'npm run setup', which should probably be ok for npm Ci, since npm install already changes the package-lock

Any suggestions on how to proceed from here? Not sure what to check/rectify

@ang-zeyu
Copy link
Contributor

npm install --save-dev lerna@v4.0.0 - the issue here might be that the updates from npm install are inconsistent / overly aggressive compared to how lerna manages its packages' dependencies. (e.g. in terms of updating transtive dependencies)
npm ci dosen't have this issue on the contrary.

Any suggestions on how to proceed from here? Not sure what to check/rectify

Would try the following:

  1. git reset --hard HEAD~1
  2. npm run setup, ensure there's no diffs (get a fresh working directory again)
  3. Bump lerna in package.json manually
  4. npx lerna bootstrap --hoist (you can also run the other commands in npm run setup except npm ci, but they should be unnecessary / no-ops)
  5. Stage/commit changes
  6. npm run updatetest + npm run setup to verify no diffs

@tlylt
Copy link
Contributor Author

tlylt commented Aug 21, 2022

Would try the following:

  1. git reset --hard HEAD~1
  2. npm run setup, ensure there's no diffs (get a fresh working directory again)
  3. Bump lerna in package.json manually
  4. npx lerna bootstrap --hoist (you can also run the other commands in npm run setup except npm ci, but they should be unnecessary / no-ops)
  5. Stage/commit changes
  6. npm run updatetest + npm run setup to verify no diffs

Thanks @ang-zeyu, updated as above.

Copy link
Contributor

@ang-zeyu ang-zeyu left a comment

Choose a reason for hiding this comment

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

Lgtm!

@ang-zeyu ang-zeyu added this to the 4.0.2 milestone Aug 22, 2022
@ang-zeyu ang-zeyu merged commit 37e6d3f into MarkBind:master Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants