-
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 to verkko v2.2.1 #51111
update to verkko v2.2.1 #51111
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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)
recipes/verkko/build.sh (1)
Line range hint
10-22
: LGTM: Overall script structure is sound.The unchanged parts of the script maintain good practices:
- OS-specific handling for macOS (Darwin) to ensure compatibility with system Boost.
- Proper build process with clean and parallel make.
- Correct installation of binaries and libraries to $PREFIX.
As a minor suggestion for improved readability, consider adding a comment before the Darwin-specific block explaining its purpose, e.g.:
# Remove built-in Boost library on macOS to use system Boost instead if [ "$(uname)" == "Darwin" ]; then ... fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/verkko/build.sh (1 hunks)
- recipes/verkko/meta.yaml (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- recipes/verkko/meta.yaml
🔇 Additional comments (3)
recipes/verkko/build.sh (3)
4-4
: LGTM: Directory creation is appropriate.Creating the
$PREFIX/bin
directory ensures that the subsequent operations have a valid target location. This is a good practice in build scripts.
6-8
: LGTM: Appropriate compilation and linking flags set.The added environment variable exports are crucial for the build process:
LDFLAGS
correctly adds the$PREFIX/lib
to the library search path.CFLAGS
andCXXFLAGS
both include the-O3
optimization flag, which is suitable for release builds.CXXFLAGS
also includes the prefix include directory in the include search path.These changes ensure that the build process uses the correct libraries and include files from the conda environment, and applies appropriate optimizations.
Line range hint
1-22
: Overall LGTM with a request for version verification.The changes to the build script are appropriate and improve the build process by setting correct compilation and linking flags. The overall structure of the script is sound and follows good practices for bioconda recipes.
However, I noticed that the PR objectives mention updating verkko to version 2.2.1, but this version update is not reflected in the build script. Could you please verify that the version update is handled correctly, possibly in other files of the PR that are not included in this review?
To verify the version update, please run the following command:
✅ Verification successful
Version update verified in
recipes/verkko/meta.yaml
.The PR correctly updates verkko to version 2.2.1 in
recipes/verkko/meta.yaml
using templating syntax.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version update in meta.yaml file # Expected result: Should show the updated version number (2.2.1) rg --type yaml 'version:\s*2\.2\.1' recipes/verkko/meta.yamlLength of output: 62
Script:
#!/bin/bash # Description: Search the entire repository for version 2.2.1 to verify the update # Expected result: Should show occurrences of '2.2.1' indicating the version update rg '2\.2\.1'Length of output: 12013
@mencian I think the issue is the change to compiler{'rust'} rather than the explicit version. For some reason that changes the build path output (see #48088). It's incorrect to use rukki 0.3.0 since the code is specifically a submodule in verkko so a specific tag was tested/built with verkko and 0.3.0 might not be it. |
Reverting to an explicit version of rust did the trick; apologies for not looking into the previous PRs. |
Updating to bug fix release of verkko v2.2.1
Summary by CodeRabbit
New Features
verkko
package to 2.2.1.Bug Fixes
Documentation