-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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 gfastats tool version #51246
Update gfastats tool version #51246
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces an update to the Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/gfastats/meta.yaml (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
recipes/gfastats/meta.yaml (2)
Line range hint
1-43
: Summary of changes and next stepsThe update of gfastats from version 1.3.7 to 1.3.8 looks good overall. Here's a summary of the review:
- The version update in line 2 is correct.
- The build number should be reset to 0.
- We need to verify that the git_tag in the source section has been updated to match the new version.
Once these points are addressed, the PR should be ready for merging.
🧰 Tools
🪛 yamllint
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
2-2
: Verify git_tag consistency with new version.The version has been updated correctly. Let's ensure that the
git_tag
in thesource
section has been updated to match.Run the following script to verify the git_tag:
✅ Verification successful
git_tag is consistent with the new version
The
git_tag
in thesource
section correctly matches the new versionv1.3.8
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that git_tag matches the new version # Test: Check if git_tag is correctly set to v1.3.8 grep -n "git_tag:" recipes/gfastats/meta.yamlLength of output: 74
Script:
#!/bin/bash # Description: Verify that the version is correctly set to 1.3.8 grep -n 'set version =' recipes/gfastats/meta.yamlLength of output: 82
@BiocondaBot please add label |
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/gfastats/meta.yaml (1)
3-3
: Remove trailing spaces.There are trailing spaces on this line. While this doesn't affect functionality, it's good practice to remove them for consistency and to avoid potential issues in some systems.
You can remove the trailing spaces from this line.
🧰 Tools
🪛 yamllint
[error] 3-3: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/gfastats/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/gfastats/meta.yaml
[error] 3-3: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
recipes/gfastats/meta.yaml (4)
2-2
: Version update looks good.The version has been correctly updated from 1.3.7 to 1.3.8, which aligns with the PR objective.
13-13
: Build number correctly reset.The build number has been properly reset to 0, which is the correct practice when updating the package version. This change addresses the concern raised in the previous review.
Line range hint
1-52
: Overall, the changes look good and align with the PR objectives.The version has been correctly updated to 1.3.8, the source URL has been adjusted accordingly, and the build number has been reset to 0 as per best practices. These changes are consistent with the PR objectives and address the concerns raised in the previous review.
A few minor points to note:
- Please verify the SHA256 checksum as suggested earlier.
- Consider removing the trailing spaces on line 3 for better code hygiene.
Once these small items are addressed, the PR should be ready for merging.
🧰 Tools
🪛 yamllint
[error] 3-3: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-10
: Source URL update is correct. Verify SHA256 checksum.The source URL has been correctly updated to use the new version 1.3.8. The change from using
git_tag
to a direct tarball URL is valid. The SHA256 checksum has also been updated, which is necessary for the new version.Please verify the SHA256 checksum to ensure it matches the new tarball. You can use the following command:
Compare the output with the SHA256 value in the meta.yaml file.
@gf777 seems that the tarball is still missing something. @SaimMomin12 sorry for the wrong advice I was hoping the tarball works now, please revert to the git checkout. |
Thanks @bgruening |
I am not sure what you want to do. This version that I recently added: wget https://github.com/vgl-hub/gfastats/releases/download/v1.3.8/gfastats.v1.3.8-with_submodules.zip compiles fine as it contains the submodule as well. Looks like it may be using the wrong version? |
The update looks good to me! A recipe could get the Git submodules by using 'git_url' with a Git tag |
@SaimMomin12 I think what happend is that you used the wrong URL, see 2242514 and https://github.com/vgl-hub/gfastats/releases/download/v1.3.8/gfastats.v1.3.8-with_submodules.zip from @gf777 .... |
@bgruening Oh no, my bad! Would it be a good idea to edit the recipe now and create a PR to correct this? |
This PR concerns with version update of gfastats tool from 1.3.7 to 1.3.8
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>
.