-
Notifications
You must be signed in to change notification settings - Fork 23
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
add Rust package guidelines #33
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces significant updates to the guidelines for creating and maintaining bioconda recipes. A new section on Rust packaging has been added, detailing the use of the Rust compiler, best practices for including licenses, and ensuring reproducibility in builds with specific Cargo flags. The C/C++ packaging section has been expanded to clarify the specification of build tools and compilers, including the use of compiler macros and examples for structuring the Changes
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 (1)
_sources/contributor/guidelines.rst.txt (1)
486-559
: Excellent addition of Rust packaging guidelines!The new Rust section is a valuable addition to the guidelines. It provides comprehensive information on compiler specification, license handling, and recommended Cargo flags for reproducible builds. The typical Rust recipe example is particularly helpful.
One suggestion for improvement:
Consider adding a brief explanation of why the
run_exports
section is included in the example recipe. This would help contributors understand when and why they should use this feature in their Rust recipes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- _sources/contributor/guidelines.rst.txt (2 hunks)
🔇 Additional comments (3)
_sources/contributor/guidelines.rst.txt (3)
Line range hint
410-485
: Improved C/C++ packaging guidelines: Great addition!The expanded C/C++ section provides clearer guidance on specifying build tools and compilers in recipes. The explanation of the
{{ compiler('c') }}
and{{ compiler('cxx') }}
macros, along with the distinction betweenbuild
andhost
dependencies, will help contributors create more accurate and portable recipes.
Line range hint
735-753
: Improved guidance on package testing: Approved!The updates to the "Tests" section provide clearer guidelines on what constitutes an adequate test for a recipe. The emphasis on avoiding the inclusion of test data in the repository is particularly important for maintaining a lean codebase.
Line range hint
1-863
: Well-structured and consistent document: Approved!The updated guidelines maintain a consistent style and structure throughout. The new sections on Rust packaging and the expanded C/C++ section integrate seamlessly with the existing content, enhancing the overall value of the document for contributors.
Don't mean to be impatient sorry, but would it be possible to get a review of this addition to the docs @bioconda/core? This is related to bioconda/bioconda-recipes#50800 |
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.
I made minor comments to make language consistent across flags; otherwise structurally this looks great. I don't know anything about rust though, so will probably need @johanneskoester to review for content.
Co-authored-by: Ryan Dale <ryan.dale@nih.gov>
Co-authored-by: Ryan Dale <ryan.dale@nih.gov>
Co-authored-by: Ryan Dale <ryan.dale@nih.gov>
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 (1)
_sources/contributor/guidelines.rst.txt (1)
486-559
: Great addition of Rust packaging guidelines!The new Rust section is comprehensive and well-structured, covering:
- Compiler specification using
{{ compiler('rust') }}
- License handling with cargo-bundle-licenses
- Recommended Cargo flags for reproducible builds
- A detailed template for a typical Rust recipe
This addition will significantly improve the quality and consistency of Rust packages in bioconda.
One suggestion for improvement:
Consider adding a brief explanation of the
run_exports
section in the template, as it might not be familiar to all contributors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- _sources/contributor/guidelines.rst.txt (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
_sources/contributor/guidelines.rst.txt (2)
Line range hint
410-485
: Excellent addition of C/C++ packaging guidelines!The new C/C++ section provides crucial information for package maintainers, including:
- Proper specification of build tools and compilers
- Use of compiler macros (e.g.,
{{ compiler('c') }}
,{{ compiler('cxx') }}
)- Distinction between build and host dependencies
This addition will greatly improve the consistency and correctness of C/C++ packages in bioconda.
Line range hint
1-559
: Overall excellent updates to the guidelines document!The changes made to this document significantly improve its comprehensiveness and usefulness for bioconda contributors. The additions of detailed C/C++ and Rust packaging guidelines are particularly valuable. The existing sections have been refined without introducing any inconsistencies.
These updates will greatly assist package maintainers in creating high-quality, consistent recipes across various programming languages and build systems.
Ping @bioconda/rust |
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!
Relates to and addresses bioconda/bioconda-recipes#50800