-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Dependencies: Upgrade commander
#28857
Conversation
Upgrades `commander` to latest version across the repo.
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 025e692. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
23 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
commander
dependencycommander
@@ -101,6 +101,7 @@ export const AppRouterProvider: React.FC<React.PropsWithChildren<AppRouterProvid | |||
childNodes: new Map(), | |||
tree, | |||
url: pathname, | |||
loading: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@valentinpalkovic I had to add this to get TS check to pass.
I think it's due to some dep getting deduped, I wanted to loop you in to have your eyes on it.
Can you check if this is an OK change?
@43081j What you think the risks of this change are? |
@ndelangen it seems fairly low. the main breaking changes seem to be that an error will be thrown if you try define an option multiple times (e.g. trying to define importantly, the changes seem to affect us rather than the consumer (i.e. they haven't changed how options are structured/parsed) |
Upgrades
commander
to latest version across the repo.If there's a particular reason we should stick to the older version, let me know and we can close this.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
N/A
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
bug
: Internal changes that fixes incorrect behavior.maintenance
: User-facing maintenance tasks.dependencies
: Upgrading (sometimes downgrading) dependencies.build
: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup
: Minor cleanup style change. Will not show up in release changelog.documentation
: Documentation only changes. Will not show up in release changelog.feature request
: Introducing a new feature.BREAKING CHANGE
: Changes that break compatibility in some way with current major version.other
: Changes that don't fit in the above categories.Greptile Summary
The pull request upgrades the
commander
dependency to the latest version across multiple files, ensuring compatibility with the new API.commander
dependency incode/core/package.json
from^6.2.1
to^12.1.0
.code/core/src/cli/bin/index.ts
to use named imports fromcommander
.code/lib/cli-storybook/package.json
andcode/lib/create-storybook/package.json
tocommander
v12.1.0.scripts/release/*.ts
files to use named imports forcommander
.commander
version.