Update trgt to 1.0.0 #50951
Update trgt to 1.0.0 #50951
Conversation
recipes/trgt/meta.yaml
Outdated
| recipe-maintainers: | ||
| - zeeev | ||
| - williamrowell | ||
|
|
||
| build: | ||
| number: 0 | ||
| noarch: generic |
There was a problem hiding this comment.
The downloaded tarball says linux-64 but here it says noarch. IMO this line should be removed
This comment was marked as resolved.
This comment was marked as resolved.
|
@BiocondaBot please add label |
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes significant updates to the Changes
Possibly related PRs
📜 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
recipes/trgt/meta.yaml (1)
10-11: Source URL and SHA256 updates look good.The source URL has been correctly updated to use the new tarball format, and the SHA256 reference is properly conditioned for the linux64 platform. This aligns with the build skip condition for non-linux64 platforms.
Consider moving the platform condition to a separate line for better readability:
source: - url: https://github.com/PacificBiosciences/trgt/releases/download/v{{ version }}/trgt-v{{ version }}-x86_64-unknown-linux-gnu.tar.gz sha256: {{ trgt_sha256 }} selector: [linux64]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/trgt/build.sh (0 hunks)
- recipes/trgt/meta.yaml (1 hunks)
💤 Files not reviewed due to no reviewable changes (1)
- recipes/trgt/build.sh
🧰 Additional context used
🪛 yamllint
recipes/trgt/meta.yaml
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (4)
recipes/trgt/meta.yaml (4)
2-2: Version update looks good.The version has been correctly updated to 1.0.0, which aligns with the PR objective.
3-3: SHA256 checksum update is correct.The SHA256 checksum has been updated to match the new version of the package.
15-15: License update is appropriate.The license has been updated to provide more specific information, including a direct link to the license file. This change improves the clarity of the licensing terms for the package.
30-30: Test command update is correct.The test command has been appropriately updated to only check the version of
trgt, which aligns with the consolidation of thetrgtandtrvzbinaries.
recipes/trgt/meta.yaml
Outdated
| extra: | ||
| recipe-maintainers: | ||
| - zeeev | ||
| - williamrowell | ||
| - tmokveld | ||
| skip-lints: | ||
| - should_be_noarch_generic |
There was a problem hiding this comment.
Extra section updates and file formatting.
The addition of the new recipe maintainer is appropriate. However, there are two issues to address:
-
As mentioned in a previous comment, the
skip-lintsentry should be removed, andnoarch: falseshould be added to thebuildsection instead. -
The file is missing a newline at the end, which is a minor formatting issue.
To resolve these issues, apply the following diff:
extra:
recipe-maintainers:
- zeeev
- williamrowell
- tmokveld
- skip-lints:
- - should_be_noarch_generic
+This change will remove the unnecessary skip-lints entry and add the required newline at the end of the file.
📝 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.
| extra: | |
| recipe-maintainers: | |
| - zeeev | |
| - williamrowell | |
| - tmokveld | |
| skip-lints: | |
| - should_be_noarch_generic | |
| extra: | |
| recipe-maintainers: | |
| - zeeev | |
| - williamrowell | |
| - tmokveld | |
🧰 Tools
🪛 yamllint
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
recipes/trgt/meta.yaml
Outdated
| skip: true # [not linux64] | ||
| script: | ||
| - mkdir -p ${PREFIX}/bin | ||
| - cp trgt ${PREFIX}/bin/ | ||
| - chmod +x ${PREFIX}/bin/trgt |
There was a problem hiding this comment.
Build script changes look good, but consider noarch package.
The build script has been correctly updated to skip non-linux64 platforms and properly handle the trgt executable. However, there's a potential issue with the package being built as platform-specific when it could be noarch.
The linter error mentioned in the PR comments suggests that this package should be built as noarch. Given that the package is a pre-built binary for a specific architecture, it cannot be noarch. To resolve this, you should:
- Remove the
skip-lintsentry forshould_be_noarch_generic. - Add
noarch: falseto thebuildsection to explicitly declare that this is not anoarchpackage.
Apply this diff to make these changes:
build:
number: 0
skip: true # [not linux64]
+ noarch: false
script:
- mkdir -p ${PREFIX}/bin
- cp trgt ${PREFIX}/bin/
- chmod +x ${PREFIX}/bin/trgt
...
extra:
recipe-maintainers:
- zeeev
- williamrowell
- tmokveld
- skip-lints:
- - should_be_noarch_genericThese changes will resolve the linter error while correctly specifying that this is a platform-specific package.
Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
@tmokveld This is the correct way ^^
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@martin-g Seems the linter isn't happy again, locally I get: ERROR: recipes/trgt/meta.yaml:27: should_be_noarch_generic: The recipe should be build as noarch
There was a problem hiding this comment.
@martin-g Is there any other suggested change to fix this? I seem to be running in circles with these changes...
There was a problem hiding this comment.
Yes, sorry about that! It seems the Bioconda build does not support it - #50985 (comment)
Please use the lint skip for now!
There was a problem hiding this comment.
Thanks, I'll do that!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/trgt/meta.yaml (1)
38-38: Minor formatting issues to address.There are two minor formatting issues detected by yamllint:
- Missing newline character at the end of the file
- Trailing spaces on the last line
To resolve these issues, please apply the following changes:
- Add a newline at the end of the file
- Remove the trailing spaces from the last line
🧰 Tools
🪛 yamllint
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
[error] 38-38: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/trgt/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/trgt/meta.yaml
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
[error] 38-38: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
recipes/trgt/meta.yaml (7)
2-2: Version update looks good.The package version has been correctly updated to 1.0.0, which aligns with the PR objective.
3-3: SHA256 checksum update is correct.The SHA256 checksum has been updated to match the new version of the package, which is crucial for ensuring the integrity of the downloaded files.
10-11: Source URL and checksum updates are correct.The source URL has been updated to reflect the new version and package format. The SHA256 checksum is correctly referenced using the Jinja2 variable. These changes are appropriately limited to the linux64 platform.
15-15: License update provides better clarity.The license has been updated to "Pacific Biosciences Software License" with a direct link to the license file. This change offers more precise information about the software's licensing terms.
20-25: Build section updates look good.The build instructions have been correctly updated to handle the linux64-specific package. The addition of
noarch: falseaddresses the linter error mentioned in the PR comments. These changes align with the suggestions made in past review comments.@martin-g The
noarch: falseaddition resolves the issue you pointed out earlier regarding the linux-64 specificity of the package.
31-31: Test command update is appropriate.The removal of the
trvztest command aligns with the PR objective to consolidate thetrgtandtrvzbinaries. The remainingtrgt --versioncommand is sufficient for testing this package.
37-37: New recipe maintainer added successfully.The addition of
tmokveldas a recipe maintainer is appropriate and aligns with the PR submitter.
Update trgt to 1.0.0. Added corresponding changes to manage consolidation of trgt and trvz binaries.
Summary by CodeRabbit
New Features
trgtpackage to version 1.0.0 with a new source URL format.trgtpackage.Bug Fixes
Chores
build.shscript, impacting the automated setup process for binaries.