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

fix(version): option to not ignore scripts on lock update #3823

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

matheo
Copy link
Contributor

@matheo matheo commented Sep 1, 2023

Description

Option to not pass --ignore-scripts while updating the lock file after the version bump.

Motivation and Context

Currently I'm using the old lerna bootstrap and I noticed that when versioning/publishing the npm lock file gets incomplete as lerna version is ignoring the scripts by default while updating it.

How Has This Been Tested?

I've used patch-package to remove this code locally and it works as expected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@matheo
Copy link
Contributor Author

matheo commented Sep 1, 2023

@fahslaj here a new small one
it is super hard to pick a name for the option when ignore-scripts already exists
and I wanted the new option to be next to --npm-client-args in the docs 😅

@fahslaj
Copy link
Contributor

fahslaj commented Sep 1, 2023

Hi @matheo , I think something like --run-scripts-on-lockfile-update would be a much more clear name for the argument. The current --no-scripts-ignore-lock is a bit ambiguous. You can always add a line to the readme near the npmClientArgs option that mentions and links to the new option, so someone looking at npmClientArgs can find the new one easily.

@matheo
Copy link
Contributor Author

matheo commented Sep 1, 2023

@fahslaj haha, the commit name was clearer indeed! thanks for the feedback!

@@ -1703,6 +1706,10 @@
"type": "boolean",
"description": "During `lerna version`, when true, commit and tag version changes."
},
"runScriptsOnLockfileUpdate": {
"type": "boolean",
"description": "During `lerna version`, when true, updates the lock file running the lifecycle scripts after the version bump."
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please clarify this description a bit? "updates the lock file running the lifecycle scripts" is unclear. Maybe something like ... when true, runs lifecycle scripts when syncing the lock file after the version bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@fahslaj fahslaj left a comment

Choose a reason for hiding this comment

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

Thank you @matheo !

@fahslaj fahslaj merged commit 4843c3c into lerna:main Sep 6, 2023
13 checks passed
@matheo matheo deleted the fix/sync-lock-with-scripts branch September 8, 2023 21:12
@matheo
Copy link
Contributor Author

matheo commented Sep 8, 2023

@fahslaj thanks!
any estimation for the next release?

@fahslaj
Copy link
Contributor

fahslaj commented Sep 13, 2023

@matheo Lerna 7.3.0 was just released and it includes these changes. Thank you!

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