-
Notifications
You must be signed in to change notification settings - Fork 207
Feature/dojoup update command #3251
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
Feature/dojoup update command #3251
Conversation
- Remove build instructions for Katana and Torii from main repo - Add references to separate Katana and Torii repositories - Update development setup to focus on core Dojo components - Clarify contribution guidelines for split architecture - Add guidance on which repository to use for different issues Fixes dojoengine#3228
- Add update command that updates to latest or specific version - Reuse existing install infrastructure for reliability - Handle already installed versions efficiently - Include comprehensive help and error handling - Support version formats: 1.5.0, v1.5.0, and 'stable' Fixes dojoengine#3229 Usage: dojoup update # Update to latest stable dojoup update 1.5.0 # Update to specific version dojoup update --help # Show help
- Add update command that updates to latest or specific version - Reuse existing install infrastructure for reliability - Handle already installed versions efficiently - Include comprehensive help and error handling - Support version formats: 1.5.0, v1.5.0, and 'stable' Fixes dojoengine#3229 Usage: dojoup update # Update to latest stable dojoup update 1.5.0 # Update to specific version dojoup update --help # Show help
|
Please review my PR @kronosapiens |
WalkthroughOhayo sensei! This update introduces major revisions to both the contribution and development guides to reflect Dojo's new multi-repository architecture and workflows. It also adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dojoup CLI
User->>Dojoup CLI: dojoup update [version]
Dojoup CLI->>Dojoup CLI: Parse version argument
Dojoup CLI->>Dojoup CLI: Determine current and target versions
alt Target version == current version
Dojoup CLI-->>User: Report already up-to-date
else Target version already installed
Dojoup CLI->>Dojoup CLI: Switch default to target version
Dojoup CLI-->>User: Report switched version
else
Dojoup CLI->>Dojoup CLI: Install target version
Dojoup CLI-->>User: Report installation complete
end
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
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: 3
🧹 Nitpick comments (2)
test_update.sh (1)
1-22: Ohayo sensei! Solid basic validation test script!The test covers the essential integration points:
- Help command functionality
- Main help includes update command
- Function presence verification
This provides good basic coverage for ensuring the update command is properly integrated.
One small suggestion - consider adding error handling for the executable permission step:
# Make executable -chmod +x dojoup/dojoup +chmod +x dojoup/dojoup || { echo "❌ Failed to make dojoup executable"; exit 1; }DEVELOPMENT.md (1)
144-160: Address the static analysis hint for compound adjective, sensei.-### Full Stack Development +### Full-Stack DevelopmentThe hyphen makes it clearer that this refers to end-to-end development spanning the full stack.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CONTRIBUTING.md(3 hunks)DEVELOPMENT.md(1 hunks)dojoup/dojoup(4 hunks)test_update.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DEVELOPMENT.md
[uncategorized] ~59-~59: You might be missing the article “the” here.
Context: ...of the different libraries that make up core Dojo - bin/: Source code of the dif...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~144-~144: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ples/spawn-and-move sozo build ``` ### Full Stack Development For end-to-end testing with...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
CONTRIBUTING.md
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...riptive title and detailed description. If possible, include a code sample or an e...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~69-~69: You might be missing the article “the” here.
Context: ...of the different libraries that make up core Dojo - bin: Source code of the diff...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~74-~74: Possible missing comma found.
Context: ...s for developers and CI Inside bin and crates you will find source code related to co...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~156-~156: You might be missing the article “the” here.
Context: ...rehensive, up-to-date documentation for Dojo. If your changes require updates to the...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~161-~161: Possible missing article found.
Context: ...ank you for considering contributing to Dojo. Your contribution is invaluable to us....
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~173-~173: Possible missing preposition found.
Context: ...and Communication ### Getting Help - Discord: Join our [Discord community](https:/...
(AI_HYDRA_LEO_MISSING_TO)
🔇 Additional comments (15)
dojoup/dojoup (5)
114-114: Ohayo sensei! LGTM on adding the update command to global usage.The integration looks clean and follows the existing pattern.
507-531: Excellent command-line argument processing pattern, sensei!The argument processing follows the same robust pattern used in
install_dojo(), with proper help flag handling and version parameter extraction.
585-603: Ohayo! Excellent help documentation, sensei!The usage examples are comprehensive and clear. Great work providing multiple format examples and practical use cases.
973-975: Perfect integration with the main command dispatcher, sensei!The update command is properly integrated following the existing pattern.
1005-1005: Good addition of error handling to main, sensei!The
|| exit 1ensures proper error propagation from the main function.DEVELOPMENT.md (4)
3-22: Ohayo sensei! Excellent restructuring of the setup guide!The clear distinction between development setup vs. user installation is very helpful, and the prerequisites section is comprehensive.
26-45: Great dual installation approach documentation, sensei!Providing both dojoup and source installation options gives developers flexibility. The commands are accurate and well-formatted.
75-97: Solid testing documentation with nextest, sensei!The testing instructions are clear and the CI command replication is very helpful for developers.
162-167: Excellent cross-repository contribution guidance, sensei!This clearly directs contributors to the appropriate repositories for different types of issues. Very helpful for the multi-repo architecture!
CONTRIBUTING.md (6)
7-36: Ohayo sensei! Outstanding repository architecture documentation!The table-based approach makes it crystal clear which repository to contribute to for different types of changes. This will significantly reduce confusion for new contributors.
44-51: Excellent issue reporting guidelines with clear mapping, sensei!The table format makes it easy for contributors to know where to report different types of issues. Very well organized!
80-83: Important architectural clarification, sensei!Clearly calling out that Katana and Torii are now in separate repositories prevents confusion and misdirected contributions.
120-127: Good clarification on Katana testing requirements, sensei!This accurately reflects the new multi-repository reality where Katana needs to be obtained separately for testing.
137-153: Excellent cross-repository contribution workflow, sensei!This provides practical guidance for complex features that span multiple repositories. The coordination process is well thought out.
170-184: Great community engagement section, sensei!The recognition mechanisms and clear communication channels will help build a stronger contributor community.
|
Dojoup will be deprecated in favor of Thank you @ritik4ever for the efforts here. 🫡 |
Summary
Implements the
dojoup updatecommand as requested in #3229.Problem Solved
Previously, updating Dojo required manual steps:
rm -rf ~/.dojodojoupagainNow users can simply run:
dojoup updateImplementation Details
1.5.0,v1.5.0, andstableUsage Examples