-
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
Update annonars to 0.42.3 #51210
base: master
Are you sure you want to change the base?
Update annonars to 0.42.3 #51210
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request consist of modifications to two files: In 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 (
|
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
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)
recipes/annonars/meta.yaml (1)
53-54
: Consider future platform supportThe commented-out section for additional platforms (specifically linux-aarch64) has been reformatted. While this change doesn't affect the current functionality, it raises a question:
Is there a plan to support linux-aarch64 in the future? If so, it might be worth adding a TODO comment or creating an issue to track this potential enhancement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/annonars/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/annonars/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (6)
recipes/annonars/meta.yaml (6)
1-1
: LGTM: Version updateThe package version has been correctly updated to 0.41.2, which aligns with the PR objectives.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-9
: Please clarify the reason for skipping macOS buildsThe addition of
skip: True # [osx]
indicates that this package will not be built for macOS. Could you please provide the rationale behind this decision? Is there a known compatibility issue or a specific reason for excluding macOS builds?
21-21
: LGTM: Build requirement updatesThe change to use
{{ compiler('rust') }}
instead of a specific Rust version provides more flexibility for the build process. This is a good practice.New dependencies have been added: libprotobuf, rocksdb, and snappy. These additions suggest changes in the annonars package requirements. Could you please provide more context on why these new dependencies are necessary? Are they related to new features or optimizations in annonars 0.41.2?
Also applies to: 25-27
30-30
: LGTM: Host requirement updateThe addition of libprotobuf to the host requirements is consistent with its addition to the build requirements. This ensures that the protobuf library is available during both build and runtime, which is correct.
44-44
: LGTM: Improved package metadataThe additions to the
about
section enhance the package metadata:
- The
license_family: APACHE
provides clear information about the license type.- The
dev_url
pointing to the GitHub repository is helpful for users who want to access the source code or contribute to the project.These changes improve the package documentation and are consistent with the project information.
Also applies to: 47-47
15-15
: Verify SHA256 checksumThe SHA256 checksum has been updated, which is expected with the new version. To ensure the integrity of the package, please confirm that this checksum matches the official release of annonars v0.41.2.
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)
recipes/annonars/meta.yaml (1)
21-21
: Build requirement changes look good, consider updating documentation.The Rust compiler requirement has been updated to use Conda's compiler syntax, which provides more flexibility. New build requirements (libprotobuf, rocksdb, and snappy) have been added, which likely reflect changes in the package's dependencies.
These changes appear correct, but it would be beneficial to update the package documentation to reflect these new dependencies, if not already done.
Also applies to: 25-27
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/annonars/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/annonars/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (6)
recipes/annonars/meta.yaml (6)
1-1
: Version update looks good.The package version has been correctly updated to "0.41.2" as per the PR objectives.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-9
: Clarify the reason for skipping OSX builds.A build skip condition for OSX has been added. Could you please provide more context on why this package should not be built for OSX? This information would be helpful for maintainers and users.
30-30
: Host and run requirement changes look good, verify runtime dependencies.The addition of libprotobuf to host requirements and rocksdb to both host and run requirements is consistent with the build requirement changes. These updates appear to be correct.
To ensure completeness, please verify that all runtime dependencies are correctly listed in the run requirements section.
Also applies to: 32-32, 36-36
46-46
: License family addition is appropriate.The addition of the "APACHE" license family is consistent with the existing Apache-2.0 license and provides more clarity about the package's licensing. This is a good improvement to the metadata.
49-49
: Development URL addition is beneficial.The addition of the development URL, pointing to the GitHub repository, improves the package metadata. This information will be helpful for users and maintainers who want to access the source code or contribute to the project.
15-15
: Verify the updated SHA256 checksum.The SHA256 checksum has been updated for the new version. Please ensure that this checksum is correct for the v0.41.2 source archive.
Hm, annonars is built successfully, but execution fails: https://dev.azure.com/bioconda/bioconda-recipes/_build/results?buildId=71346&view=logs&j=e14e69ff-a0ae-55c4-b71d-229b239cfb2f&t=7df82132-b284-504b-53d6-7d3e63519572&l=1498 |
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 (2)
recipes/annonars/meta.yaml (2)
9-9
: Consider documenting OSX skip reasonThe recipe skips OSX builds, but the reason isn't documented. Adding a comment would help maintainers understand the limitation.
- skip: True # [osx] + # Skip OSX builds due to [specific reason] + skip: True # [osx]
Line range hint
55-57
: Review platform support strategyThe commented-out additional platform support for linux-aarch64 suggests potential cross-platform compatibility. Given the RocksDB dependency issues, it might be worth documenting the platform support strategy.
Consider:
- Documenting why aarch64 support is commented out
- Creating an issue to track platform support expansion
- Adding test cases for supported platforms
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/annonars/meta.yaml
(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/annonars/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/annonars/meta.yaml (2)
21-21
: Dependency changes require verification
The changes include:
- Moving from
rust >=1.30
to{{ compiler('rust') }}
- Replacing
protobuf
withlibprotobuf
These changes might affect the build process and runtime behavior.
Let's verify the impact:
Also applies to: 25-25, 30-30
✅ Verification successful
Based on the search results, I can see that libprotobuf
is used as a dependency in multiple recipes including annonars
, usher
, and homopolish
. The change from protobuf
to libprotobuf
appears to be a common pattern in the bioconda ecosystem.
Dependency change is appropriate and follows ecosystem patterns
The change from protobuf
to libprotobuf
is consistent with other bioconda packages and represents an improvement in dependency specification. The libprotobuf
package provides the actual protocol buffer runtime library that is needed, while protobuf
is a more generic package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other recipes use the new rust compiler syntax
rg "compiler\('rust'\)"
# Look for patterns of libprotobuf usage in other recipes
rg "libprotobuf" recipes/*/meta.yaml
Length of output: 324715
25-27
: Review RocksDB integration approach
The addition of RocksDB as a dependency across build, host, and run requirements aligns with the package needs, but there's a reported execution failure related to using conda-forge's RocksDB package instead of building from source.
Let's verify the RocksDB package compatibility:
Consider one of these approaches:
- Build RocksDB from source instead of using the conda-forge package
- Pin to a specific working version of RocksDB if available
- Document the specific RocksDB requirements in the recipe
Also applies to: 30-30, 32-32, 36-36
recipes/annonars/meta.yaml
Outdated
@@ -1,35 +1,39 @@ | |||
{% set version = "0.41.0" %} | |||
{% set version = "0.42.0" %} |
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.
Version mismatch with PR objectives
The version is being updated to 0.42.0, but the PR objectives indicate this should be an update to 0.41.3. This appears to be a significant version jump that might introduce breaking changes.
Please verify if this is intentional or if the version should be corrected to 0.41.3.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
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 (1)
recipes/annonars/meta.yaml (1)
Line range hint
39-41
: Consider adding more comprehensive testsGiven the reported execution issues, consider adding more thorough tests that:
- Verify RocksDB functionality
- Test basic data operations
- Validate library loading
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/annonars/meta.yaml
(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/annonars/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/annonars/meta.yaml (2)
21-21
: LGTM! Improved Rust compiler specification
Using {{ compiler('rust') }}
is the recommended approach as it allows conda to manage compiler compatibility.
1-1
:
Version mismatch with PR objectives
The PR objectives indicate updating to version 0.42.0, but the recipe shows version 0.42.1. This discrepancy needs to be resolved.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
- rocksdb | ||
- snappy |
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.
Address RocksDB execution issues
Based on the PR comments, there are execution failures related to using conda-forge's RocksDB package. Consider either:
- Compiling RocksDB from source instead of using the conda package
- Pinning to a specific working version of RocksDB
Would you like assistance in implementing either of these solutions?
Also applies to: 32-32, 36-36
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/annonars/meta.yaml
(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/annonars/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/annonars/meta.yaml (4)
1-1
: Version mismatch with PR objectives
The version is being updated to 0.42.2, but the PR objectives indicate this should be an update to 0.42.1. Please verify if this version bump is intentional.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
14-15
: LGTM!
The source configuration is properly updated with the new checksum.
21-21
: LGTM! Improved compiler specification
Using {{ compiler('rust') }}
is more maintainable than pinning to a specific Rust version.
46-49
: LGTM! Improved package metadata
Good additions of license family and development URL that enhance package documentation.
- libprotobuf | ||
- rocksdb | ||
- snappy |
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.
Critical: Address RocksDB execution issues
Based on the PR comments, there are execution failures related to using conda-forge's RocksDB package. Consider either:
- Compiling RocksDB from source instead of using the conda package
- Investigating if specific version constraints on RocksDB and its dependencies (snappy) would resolve the issue
Would you like assistance in implementing either of these solutions?
Update
annonars
: 0.41.0 → 0.42.3recipes/annonars
(click to view/edit other files)@varfish-org
This pull request was automatically generated (see docs).