-
-
Notifications
You must be signed in to change notification settings - Fork 106
chore: misc CLI fixes #1939
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
chore: misc CLI fixes #1939
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several modifications across multiple files in the ZenStack CLI package, focusing on improving version handling, error management, and user guidance. The changes primarily involve adding null checks when processing package versions, updating function return types, and modifying console output messages. These updates aim to enhance the robustness of version detection and provide clearer documentation guidance for users initializing or working with ZenStack projects. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/schema/src/utils/version-utils.ts (1)
2-2
: Explicitly declaring return type improves clarity.Declaring
string | undefined
in the function signature helps clarify the usage and ensure that consumers handle the possibility of an undefined result. Since this function reads from different files, consider logging or handling the case when neither package file is found so that users have more insight into why the version is undefined.packages/schema/src/cli/actions/init.ts (1)
66-66
: Documentation link update enhances user onboarding.Replacing the modeling guide with a direct link to the documentation is helpful for a broader overview. Consider referencing specific relevant pages or guides if advanced usage instructions are needed.
packages/schema/src/cli/cli-util.ts (2)
236-236
: Caution on silent fallback.
Returning an empty array might mask errors if the project remains misconfigured. Consider logging a warning or rethrowing an informative exception to aid debugging.} catch { - return []; + console.warn('Could not read package.json properly. Returning empty package list.'); + return []; }
289-289
: Improve user guidance by suggesting how to upgrade
When informing users of a newer version, you might also include quick instructions or references for upgrading, enhancing the user experience and making it easier to stay up to date.if (latestVersion && currVersion && semver.gt(latestVersion, currVersion)) { + console.log(`Upgrade with: npm install -g zenstack@${latestVersion}`); console.log(`A newer version ${colors.cyan(latestVersion)} is available.`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/schema/src/cli/actions/generate.ts
(1 hunks)packages/schema/src/cli/actions/info.ts
(1 hunks)packages/schema/src/cli/actions/init.ts
(1 hunks)packages/schema/src/cli/cli-util.ts
(3 hunks)packages/schema/src/cli/index.ts
(1 hunks)packages/schema/src/utils/version-utils.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (5)
packages/schema/src/cli/actions/info.ts (1)
19-21
: Good null check before using version.By checking
if (version)
before adding it toversions
, you avoid inserting an undefined value. This is a sensible approach that prevents inconsistent data in your version set.packages/schema/src/cli/actions/generate.ts (1)
40-41
: Filtering out undefined versions is a robust approach.Using
.filter((v): v is string => !!v)
ensures that only valid string versions make it into the set. This avoids potential type errors down the line. Great attention to detail!packages/schema/src/cli/index.ts (1)
76-76
: Consider handling null or undefined returns from getVersion() without forcing non-null.
While the!
operator suppresses potential undefined behavior, it might be safer to handle the case whengetVersion()
returnsundefined
. This can prevent unexpected runtime errors if the version cannot be resolved.Would you like me to provide a snippet that gracefully handles
undefined
and falls back to a default value?packages/schema/src/cli/cli-util.ts (2)
230-230
: Good approach on always returning an array.
Explicitly returning an array rather than possiblyundefined
helps keep consumption logic simpler. This ensures robust handling of unexpected file reads.
248-248
: Verify the version type at runtime.
While castingrequire(resolved).version
as a string may be accurate, consider asserting its validity or falling back toundefined
if the field doesn’t exist. This helps avoid runtime type mismatches.- return { pkg, version: require(resolved).version as string }; + const loadedVersion = require(resolved).version; + return { pkg, version: typeof loadedVersion === 'string' ? loadedVersion : undefined };
No description provided.