Add new package kestrel in version 1.0.3 #51810
Conversation
📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces two new files in the Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (1)
recipes/kestrel/build.sh (1)
9-9: Remove trailing empty lines.
The script has unnecessary empty lines at the end.
Remove the extra newlines at the end of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/kestrel/build.sh (1 hunks)
- recipes/kestrel/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/kestrel/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[warning] 37-37: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (4)
recipes/kestrel/meta.yaml (4)
16-17: Review run_exports configuration.
The run_exports section with max_pin="x" will only allow updates to minor and patch versions. Given this is a new package, please confirm if this is the intended pinning strategy for maintaining ABI compatibility.
19-25: Consider adding build requirements.
While runtime requirements are specified, build requirements might be needed if any compilation or setup is required during the package installation.
#!/bin/bash
# Check if there's a setup.py or similar build script in the source
curl -sL "https://github.com/paudano/kestrel/releases/download/1.0.3/kestrel-1.0.3-linux.tar.gz" | tar tz | grep -E "setup\.py|configure|CMakeLists\.txt|Makefile"34-35: Verify license information.
Please ensure:
- The license is correctly specified as "GPL-3.0" (standard SPDX identifier)
- The COPYING file is actually included in the source distribution
#!/bin/bash
# Check if COPYING file exists in the release tarball
curl -sL "https://github.com/paudano/kestrel/releases/download/1.0.3/kestrel-1.0.3-linux.tar.gz" | tar tz | grep -i "copying\|license"Apply this diff to use the standard SPDX identifier:
- license: GPL 3
+ license: GPL-3.08-10:
Update SHA256 checksum for version 1.0.5.
The SHA256 checksum needs to be updated to match the 1.0.5 release tarball.
recipes/kestrel/build.sh
Outdated
| #!/bin/bash | ||
|
|
||
| kestrel=$PREFIX/opt/$PKG_NAME-$PKG_VERSION | ||
| mkdir -p $kestrel | ||
| cp -r ./* $kestrel | ||
| chmod +x $kestrel/kestrel | ||
| mkdir -p $PREFIX/bin | ||
| ln -s $kestrel/kestrel $PREFIX/bin/kestrel | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Version inconsistency confirmed: meta.yaml shows 1.0.3 while PR targets 1.0.5
The meta.yaml file explicitly sets the version to 1.0.3:
{% set version = "1.0.3" %}This needs to be updated to match the intended version 1.0.5 mentioned in the PR objectives.
🔗 Analysis chain
Verify package version consistency.
The AI summary mentions version 1.0.3, but the PR objectives state version 1.0.5.
Let's verify the version in meta.yaml:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package version in meta.yaml
grep "version:" recipes/kestrel/meta.yaml
Length of output: 66
Script:
#!/bin/bash
# Let's check the package version from multiple sources
cat recipes/kestrel/meta.yaml
Length of output: 1023
recipes/kestrel/meta.yaml
Outdated
| test: | ||
| commands: | ||
| - fastqc | ||
| - fastqc -h |
There was a problem hiding this comment.
Incorrect test commands.
The test commands are checking for fastqc which is a different tool. The tests should verify the kestrel executable instead.
Replace with appropriate test commands:
test:
commands:
- - fastqc
- - fastqc -h
+ - kestrel --version
+ - kestrel --help📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test: | |
| commands: | |
| - fastqc | |
| - fastqc -h | |
| test: | |
| commands: | |
| - kestrel --version | |
| - kestrel --help |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/kestrel/meta.yaml (1)
27-30: Consider adding more comprehensive tests.While basic command presence tests are included, consider adding:
- Version output verification
- Basic functionality test with sample data
test: commands: - kestrel - - kestrel -h + - kestrel -h + - kestrel --version | grep {{ version }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/kestrel/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/kestrel/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[warning] 37-37: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (5)
recipes/kestrel/meta.yaml (5)
1-2: Version mismatch with PR objective still present.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
19-25: LGTM! Well-documented requirements section.
The OpenJDK version requirement is properly specified with good documentation explaining the channel constraint.
32-36: Verify license file presence.
The meta.yaml references a COPYING file for the license.
16-17: Consider if strict version pinning is necessary.
The run_exports section with pin_subpackage will enforce strict version matching. For a new package, this might be unnecessarily restrictive unless there are specific ABI compatibility concerns.
9-10: Verify SHA256 checksum for the package.
The checksum should be verified against the official release to ensure package integrity.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
holtgrewe
changed the title
|
@BiocondaBot please add label |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
recipes/kestrel/build.sh (1)
9-9: Add version verification after installation.Consider adding a version check after installation to ensure the correct version is installed and executable.
chmod +x "${kestrel}/kestrel" +# Verify installed version +if ! "${kestrel}/kestrel" --version 2>&1 | grep -q "${PKG_VERSION}"; then + echo "Error: Installed version does not match expected version ${PKG_VERSION}" + exit 1 +fi
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/kestrel/meta.yaml (1)
27-30: Consider adding version check to test commands.While the current tests verify basic command availability, adding a version check would ensure the correct version is installed.
test: commands: - kestrel - - kestrel -h + - kestrel -h + - kestrel --version # Add version check
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/kestrel/meta.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/kestrel/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[warning] 38-38: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (4)
recipes/kestrel/meta.yaml (4)
12-18: LGTM! Build configuration follows best practices.
The build configuration is well-structured:
noarch: genericis appropriate for Java applications- Version pinning through
run_exportsensures ABI compatibility
19-25: LGTM! Well-documented requirements.
The openjdk requirement is properly specified with:
- Clear version constraint
- Excellent documentation explaining the version choice
- Proper channel enforcement
1-38: LGTM! Well-structured Bioconda recipe.
The recipe follows Bioconda best practices with:
- Proper templating
- Clear documentation
- Appropriate dependencies
- Necessary metadata
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[warning] 38-38: too many blank lines
(1 > 0) (empty-lines)
8-10: Verify source URL and checksum.
The URL structure looks correct, but let's verify its accessibility and the SHA256 checksum.
| about: | ||
| home: 'https://github.com/paudano/kestrel' | ||
| license: GPL-3.0-or-later | ||
| license_family: GPL3 | ||
| license_file: COPYING | ||
| summary: 'Mapping-free variant caller for short-read Illumina data' |
There was a problem hiding this comment.
💡 Codebase verification
Homepage is accessible but COPYING file is missing
The homepage URL is valid and accessible (HTTP 200 response). However, I don't see a COPYING file in the kestrel recipe directory, which is required as specified in the meta.yaml. Other license files exist in various recipe directories, but not for kestrel.
🔗 Analysis chain
Verify license file and homepage.
The about section looks good, but let's verify:
- The existence of the COPYING file in the source
- The accessibility of the homepage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify homepage accessibility
curl -sI 'https://github.com/paudano/kestrel' | head -n 1
# Note: Can't verify COPYING file without downloading and extracting the source
Length of output: 69
Script:
#!/bin/bash
# Search for COPYING file in the repository
fd "COPYING" --type f --hidden
# Check if there's any other license file that might be renamed
fd -i "license|copying" --type f --hidden
Length of output: 8481
|
@berntpopp - this might come in handy |
Describe your pull request here
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein 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_exportssection 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 eithernameif aname|lowervariable 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 labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand 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>.