-
-
Notifications
You must be signed in to change notification settings - Fork 635
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: unintended project initialization at absolute path /trigger
during project initialization
#1412
fix: unintended project initialization at absolute path /trigger
during project initialization
#1412
Conversation
🦋 Changeset detectedLatest commit: 8a32367 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes in this pull request include updates to the CLI documentation and modifications to the Changes
Assessment against linked issues
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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
🧹 Outside diff range and nitpick comments (2)
packages/cli-v3/DEVELOPMENT.md (1)
10-10
: Approved with a suggestion for clarityThe updated description provides more specific information about the location of job-catalogs, which is helpful for users. However, to avoid potential confusion, consider clarifying whether "/references" is an absolute path or relative to the project root.
Suggestion: Consider rephrasing to "job-catalogs located in the
references
directory of the project" if it's not an absolute path.packages/cli-v3/src/commands/init.ts (1)
232-234
: LGTM! This change addresses the unintended project initialization issue.The addition of
location.replace(/^\//, "")
ensures that the path is always treated as relative, preventing unintended project initialization at the root of the drive. This effectively resolves the issue described in #1081.A minor suggestion to improve code clarity:
Consider adding a comment explaining why we're stripping the leading slash. This will help future maintainers understand the rationale behind this change. For example:
// Ensure that the path is always relative by stripping leading '/' if present -const relativeLocation = location.replace(/^\//, ""); +// This prevents unintended project initialization at the root of the drive (issue #1081) +const relativeLocation = location.replace(/^\//, "");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/cli-v3/DEVELOPMENT.md (1 hunks)
- packages/cli-v3/src/commands/init.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
packages/cli-v3/DEVELOPMENT.md (2)
Line range hint
1-14
: Documentation updates align well with PR objectivesThe changes to this file improve the clarity of instructions for running the CLI from source. They provide more specific information about job-catalog locations and reflect updates to the CLI command structure. These updates align well with the PR objectives of enhancing CLI initialization and documentation.
14-14
: Approved with a request for additional informationThe updated command structure provides more flexibility and aligns with common CLI patterns. This change improves usability and is consistent with the PR objectives.
To ensure completeness of the documentation:
- Could you provide examples of common commands that users might use?
- Is there a way to list all available commands (e.g., a
--help
option)?To verify the available commands, we can run the following script:
b3a2a25
to
8e3b8d9
Compare
b3e9319
to
711a83a
Compare
711a83a
to
8a32367
Compare
Closes #1081
✅ Checklist
Testing
[Describe the steps you took to test this change]
/references/hello-world
/trigger
input as project path/trigger
is present at/references/hello-world/trigger
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Bug Fixes
/trigger
.