-
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 el_gato to 1.20.1 #51051
Update el_gato to 1.20.1 #51051
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 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 (
|
@mencian Thanks for tidying up the recipe. The build is now failing because the database files that were being copied in the build.sh script aren't found. Are you able to offer any guidance here on how to fix this issue? Is the |
It looks to me that the DB files were created by https://github.com/bioconda/bioconda-recipes/pull/51051/files#diff-23e0b91a3b7cd25eeb68639a967975077fa68686b822522cc35970d2fed9b2dcL3 |
Yes, I saw the changes you made. However, the build now fails saying it is unable to find one of the files which should have been copied. It seems to be looking in the wrong location now:
That file should be in /opt/conda/conda-bld/el_gato.../bin/db/ Before your changes, the build failed with |
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/el_gato/meta.yaml (3 hunks)
🔇 Additional comments (5)
recipes/el_gato/meta.yaml (5)
2-3
: Version and hash update looks good.The version has been correctly updated to 1.20.0, and the corresponding SHA256 hash has been updated. This is consistent with the PR objectives.
22-22
: Run exports update is appropriate.The change in
max_pin
from 'x.x' to 'x' allows for more flexible version compatibility, which is consistent with common practices for Python packages.
49-49
: Summary update is a minor improvement.The addition of a period at the end of the summary is a small but welcome grammatical improvement.
37-38
: New dependencies added appropriately.The addition of
colorama
for Linux andimportlib-metadata
for Python <= 3.8 addresses specific requirements for different environments. The conditions are correctly specified.To ensure these dependencies are correctly utilized in the package, please run the following script:
#!/bin/bash # Description: Verify the usage of new dependencies in the package # Test: Check for imports or usage of colorama and importlib-metadata rg -i 'import\s+(colorama|importlib[-_]metadata)' --type python rg -i '(colorama|importlib[-_]metadata)\..' --type python
50-51
: New URL additions are valuable.The addition of
dev_url
anddoc_url
provides important links to development and documentation resources. The URLs are correctly formatted and use appropriate variables.To ensure these URLs are correct and accessible, please run the following script:
recipes/el_gato/meta.yaml
Outdated
noarch: python | ||
script: | ||
- {{ PYTHON }} -m pip install . -vvv --no-deps --no-build-isolation --no-cache-dir | ||
- mkdir -p $PREFIX/bin/db | ||
- cp el_gato/db/* $PREFIX/bin/db/ |
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.
💡 Codebase verification
Issue with $PREFIX
usage: Environment variable is not set.
The $PREFIX
environment variable is not set, causing the script to attempt copying database files to /bin/db/
, which does not exist.
Please ensure that $PREFIX
is correctly set before running the build script.
🔗 Analysis chain
Build configuration update looks good, but verify $PREFIX
usage.
The change to noarch: python
is appropriate for a Python package. The new build script correctly installs the package using pip and copies the database files.
However, based on the PR comments, there might be an issue with the $PREFIX
usage.
Please verify the correct path for database files using the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the correct path for database files
# Test: Check if the database files are correctly copied to the expected location
fd -t f . "$PREFIX/bin/db"
Length of output: 281
Script:
#!/bin/bash
# Description: Verify the correct path for database files
# Test: Check if the database directory exists and list its contents
if [ -d "$PREFIX/bin/db" ]; then
echo "Contents of $PREFIX/bin/db:"
ls -la "$PREFIX/bin/db/"
else
echo "Error: Directory $PREFIX/bin/db does not exist."
exit 1
fi
Length of output: 120
Script:
#!/bin/bash
# Description: Print the value of $PREFIX
echo "PREFIX is set to: $PREFIX"
Length of output: 46
* bump version to 1.20.0 and (hopefully) fix build issues in #51051 * try noarch python * build noarch generic but try skipping noarch lint * add new 1.20.1 release
Closing in favor of #51484. |
Update
el_gato
: 1.19.0 → 1.20.1recipes/el_gato
(click to view/edit other files)@appliedbinf
This pull request was automatically generated (see docs).