-
Notifications
You must be signed in to change notification settings - Fork 207
docs: update DEVELOPMENT.md and CONTRIBUTING.md after repo split #3250
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
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
|
please review @kronosapiens |
WalkthroughThe Changes
Sequence Diagram(s)No sequence diagram generated, as the changes are documentation-only and do not alter control flow or feature logic. Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Ohayo sensei! ✨ 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: 1
🧹 Nitpick comments (8)
CONTRIBUTING.md (8)
19-28: Repository Usage Instructions
The “Which Repository Should I Use?” bullets are clear. Consider swapping “Contribute here for:” to “Use this repo for:” for consistency with other guides.
53-64: Fork, Clone & Environment Setup
Suggest adding a quickgit cloneexample and a note for macOS users (e.g., Homebrew install for Rust) to smooth cross-platform onboarding.
67-73: Project Folder Structure Description
Nit: add “the” in “Source code of the different libraries that make up the core Dojo” to improve grammar:- **crates**: Source code of the different libraries that make up core Dojo + **crates**: Source code of the different libraries that make up the core Dojo
80-85: Separate Repos Important Note
You use “Important” twice (header + opening sentence). Consider merging or rephrasing the sentence to avoid redundancy.
94-113: Testing Database Extraction & Regeneration
The extraction and rebuild steps are detailed. For clarity, you could convert the two bash snippets into a numbered list of steps.
114-128: Running Tests with and without Katana
Tests instructions are solid. You might label the code block explicitly asbashand call out environment-variable usage in a note block for better visibility.
129-136: Pull Request Submission
Consider linking to any existing PR template or issue template to guide submitters on format and metadata.
155-159: Documentation Section
Good callout to the Book repo. Optionally, bold or highlight the link to the Book repo for better discoverability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CONTRIBUTING.md(3 hunks)DEVELOPMENT.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DEVELOPMENT.md
[style] ~5-~5: Consider a more concise word here.
Context: ... Please follow the following guidelines in order to streamline your contribution. It helps ...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~5-~5: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...time of the developers maintaining this open source project. In return, maintainers will re...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[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)
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] ~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)
🪛 markdownlint-cli2 (0.17.2)
DEVELOPMENT.md
30-30: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
CONTRIBUTING.md
30-30: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
🔇 Additional comments (8)
CONTRIBUTING.md (7)
7-18: ohayo sensei,
Repository Architecture Section
The new “Repository Architecture” section clearly outlines each repo’s purpose and includes valid links. No further changes needed here.
29-36: External Repositories Table
The mapping of contribution types to external repos is comprehensive and accurate. Links render correctly and examples are on point.
43-52: Issue Reporting Guidelines Table
Well-structured table; the examples align with the mapped repos. No issues spotted.
137-153: Cross-Repository Workflow
The example workflow and coordination steps are comprehensive and on point. No further edits needed.
160-169: Final Notes Clarity
The closing guidance is concise and helpful. All links and channels are correct.
170-176: Community & Communication Channels
The community links and channel descriptions are clear. No action needed here.
177-184: Contributor Recognition & Conclusion
Nice finish—contributors will appreciate the shout-outs. Everything looks good.DEVELOPMENT.md (1)
5-184: Skip remaining review
All other segments appear to duplicate CONTRIBUTING.md. Once the header/content is corrected, we can reassess the intended development instructions.
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 (2)
DEVELOPMENT.md (2)
69-72: Convert bare URLs to named links
Use reference-style links for consistency and cleaner markdown.- **Note**: Katana and Torii are now maintained in separate repositories: - - Katana: https://github.com/dojoengine/katana - - Torii: https://github.com/dojoengine/torii + **Note**: Katana and Torii are now maintained in separate repositories: + - [Katana][katana-repo] + - [Torii][torii-repo] [katana-repo]: https://github.com/dojoengine/katana [torii-repo]: https://github.com/dojoengine/torii
75-78: Hyperlink Nextest URL
Convert the bare URL to a descriptive link for consistency.- We use `nextest` as our test runner. Install it from https://nexte.st/. + We use `nextest` as our test runner. Install it from [Nextest][nextest]. [nextest]: https://nexte.st/
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
DEVELOPMENT.md(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)
🪛 markdownlint-cli2 (0.17.2)
DEVELOPMENT.md
70-70: Bare URL used
null
(MD034, no-bare-urls)
71-71: Bare URL used
null
(MD034, no-bare-urls)
75-75: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (19)
DEVELOPMENT.md (19)
3-3: Ohayo sensei,
Introduction updated correctly
The new introduction clearly distinguishes development setup from end-user installation workflows.
5-8: Repository structure section adds clarity
The new section clearly outlines the multi-repo architecture and sets context for contributors.
9-12: Repository list is well-formatted
Hyperlinks to the Katana and Torii repositories help navigate quickly.
15-23: Prerequisites section is clear
Listing core dependencies and linking to the installation guide covers setup needs comprehensively.
24-32: Option 1 installation instructions are concise
Using thedojoupinstaller simplifies setup for new contributors.
34-45: Source build instructions are accurate
Cloning and installing Katana and Torii from source is documented clearly.
47-53: Environment setup steps are straightforward
The clone command and section structure guide contributors effectively through initial setup.
55-63: Architecture overview is detailed
Folder breakdown and example locations help contributors orient themselves in the codebase.
64-68: Key components section is informative
Listingsozo,dojo/core, anddojo/langclarifies core crate responsibilities.
79-88: Test command examples are comprehensive
Commands for preparing artifacts and running selective tests cover common scenarios.
92-97: Local test guidance aligns with CI
Providing the exact CI command helps reduce friction for contributors.
101-113: Rebuild artifacts method 1 is clear
Describes using system-installed Katana and scripts effectively.
114-122: Rebuild artifacts method 2 details are accurate
Building Katana from source and copying binaries is well documented.
124-127: Temporary path note is helpful
Explaining/tmp/conventions prevents confusion about binary locations.
128-142: Core development workflow is well-structured
Commands for building, testing, and running examples guide typical contributor flow.
144-160: Full-stack development instructions are comprehensive
Parallel terminal setup for Katana, Sozo, Torii, and client app covers end-to-end testing well.
162-167: Cross-repo contribution links are clear
Direct links to related repositories help contributors route issues correctly.
170-173: Releasing instructions are precise
Describing GitHub action triggers and expected outcomes aligns with CI workflows.
176-178: Getting help section is concise
Offering documentation, community links, and issue guidance rounds out the guide nicely.
|
Thanks for the suggestions @ritik4ever bht it feels a bit off at some places and much repetitions for not much information added. The repository split is great and maybe @kronosapiens the work could be articulated around that. |
|
Will close if you have new feedback please don't hesitate to discuss them and re-open PR as necessary. |
Fixes #3228
Description
Related issue
Tests
Added to documentation?
Checklist
scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit
Summary by CodeRabbit