-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Adding the devider recipe #51365
Adding the devider recipe #51365
Conversation
Co-authored-by: Michael Hall <[email protected]>
Co-authored-by: Michael Hall <[email protected]>
Co-authored-by: Michael Hall <[email protected]>
Co-authored-by: Michael Hall <[email protected]>
Co-authored-by: Michael Hall <[email protected]>
📝 WalkthroughWalkthroughThis pull request introduces two new files in the Possibly related PRs
Suggested labels
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (5)
recipes/devider/build.sh (3)
1-1
: Consider adding 'pipefail' to shebang options for improved error handling.The current shebang includes good practices for error handling with
-e
and-u
. However, addingpipefail
would further enhance error detection by causing a pipeline to return the exit status of the last command to exit with a non-zero status.Consider updating the shebang as follows:
-#!/bin/bash -euo +#!/bin/bash -euo pipefailThis change will ensure that errors in pipelines are not masked by the success of subsequent commands.
3-5
: Approve Git connection workaround with a minor suggestion.The workaround for SSH-based Git connections is well-documented and necessary. Good job on providing context with the GitHub issue reference.
Consider splitting the export statement into separate lines for better readability and to avoid potential issues with return value masking:
-export CARGO_NET_GIT_FETCH_WITH_CLI=true CARGO_HOME="$(pwd)/.cargo" +export CARGO_NET_GIT_FETCH_WITH_CLI=true +export CARGO_HOME="$(pwd)/.cargo"This change addresses a potential shellcheck warning and improves code clarity.
🧰 Tools
🪛 Shellcheck
[warning] 5-5: Declare and assign separately to avoid masking return values.
(SC2155)
7-9
: Approve Rust binary build process with a suggestion for improved error handling.The build process is well-structured, using appropriate cargo flags and including license bundling. Good job on ensuring reproducible builds with the
--locked
flag and settingRUST_BACKTRACE=1
for better debugging.Consider adding error checking after the
cargo-bundle-licenses
andcargo install
commands to ensure the script fails if these critical steps encounter issues:# build statically linked binary with Rust -cargo-bundle-licenses --format yaml --output THIRDPARTY.yml +cargo-bundle-licenses --format yaml --output THIRDPARTY.yml || { echo "Error: Failed to bundle licenses"; exit 1; } -RUST_BACKTRACE=1 cargo install --verbose --locked --no-track --root $PREFIX --path . +RUST_BACKTRACE=1 cargo install --verbose --locked --no-track --root $PREFIX --path . || { echo "Error: Failed to install package"; exit 1; }This change will make the script more robust by explicitly handling potential failures in these critical steps.
recipes/devider/meta.yaml (2)
16-29
: LGTM: Requirements are well-defined, but there's a minor formatting issue.The build and run requirements are correctly specified and consistent with the package needs. However, there's a minor formatting issue:
Remove the trailing space on line 24:
- - python + - python🧰 Tools
🪛 yamllint
[error] 24-24: trailing spaces
(trailing-spaces)
37-43
: LGTM: About section is comprehensive, but there's a minor formatting issue.The about section provides all necessary information, including the home page, license, license files, and a clear summary of the package's functionality. However, there's a minor formatting issue:
Remove the trailing space on line 42:
- - THIRDPARTY.yml + - THIRDPARTY.yml🧰 Tools
🪛 yamllint
[error] 42-42: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/devider/build.sh (1 hunks)
- recipes/devider/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/devider/build.sh
[warning] 5-5: Declare and assign separately to avoid masking return values.
(SC2155)
🪛 yamllint
recipes/devider/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
recipes/devider/meta.yaml (6)
1-5
: LGTM: Package and version are correctly defined.The package name and version are consistent with the PR objectives. Using Jinja2 templating for the version is a good practice for maintainability.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
11-14
: LGTM: Build section is correctly configured.The build number is appropriately set to 0 for a new package. The
run_exports
section withpin_subpackage
is correctly implemented, which will help manage dependency versions as suggested in the PR objectives.
31-35
: LGTM: Test commands are appropriate.The test commands cover the main executable and associated scripts, which is good for ensuring proper installation. Using help commands (-h) is a simple yet effective way to verify that the executables are accessible and runnable.
45-47
: LGTM: Maintainer information is correctly specified.The recipe maintainer is properly set to the PR submitter (bluenote-1577), which is consistent with the PR objectives.
1-47
: Overall, the meta.yaml file is well-structured and complete.The file contains all necessary information for the devider package, including version, source, build and run requirements, test commands, license information, and maintainer details. The content is consistent with Bioconda recipe guidelines and the PR objectives.
Minor formatting issues (trailing spaces) were identified and should be addressed.
As a final verification step, please run the following command to ensure the YAML file is valid and free of syntax errors:
#!/bin/bash # Validate the YAML file python -c "import yaml; yaml.safe_load(open('recipes/devider/meta.yaml'))"If this command runs without errors, it confirms that the YAML file is syntactically correct.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
7-9
: LGTM: Source URL and checksum are correctly specified.The source URL and SHA256 checksum are properly defined. However, it's always good to double-check the checksum for accuracy.
To verify the checksum, you can run the following command:
Compare the output with the checksum in the meta.yaml file to ensure they match.
This is a PR to add the devider long-read haplotyping package : https://github.com/bluenote-1577/devider
This is a rename of the previous
dbghaplo
recipe (https://github.com/bioconda/bioconda-recipes/tree/master/recipes/dbghaplo) I added to bioconda.I opted for adding a new repo for this renaming. Let me know if another procedure is preferred.
The recipe should be exactly the same as the previous dbghaplo recipe. Previous PR here: #51035
Thank you!
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add label
command.@bioconda/core
in a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports
(see here for the rationale and comprehensive explanation).Add a
run_exports
section like this:with
...
being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}
while replacing
"myrecipe"
with eithername
if aname|lower
variable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update
@BiocondaBot please add label
please review & merge
label.@BiocondaBot please fetch artifacts
You can use this to test packages locally.
Note that the
@BiocondaBot please merge
command is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>
and will automatically re-post them to notify the addressed<team>
.