-
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 version to 1.20.1 #51484
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request involves significant updates to the Possibly related PRs
Suggested labels
Suggested reviewers
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 (2)
recipes/el_gato/meta.yaml (2)
17-20
: Build script changes look good, with a minor suggestion.The new build script effectively replaces the deleted build.sh file. It correctly sets up the database files and installs the package using pip with appropriate flags.
Consider using
$PREFIX/share
instead of$PREFIX/bin
for storing the database files, as it's more appropriate for non-executable data:- - mkdir -p $PREFIX/bin/db - - cp -rf el_gato/db/* $PREFIX/bin/db/ + - mkdir -p $PREFIX/share/el_gato/db + - cp -rf el_gato/db/* $PREFIX/share/el_gato/db/Don't forget to update any references to this path in the package code if you make this change.
26-26
: Requirements changes look good, with a minor suggestion.The addition of Python as a build requirement is appropriate. The new run requirements for colorama (Linux only) and importlib-metadata (Python <= 3.8) address platform-specific needs.
Consider specifying a minimum version for colorama and importlib-metadata to ensure compatibility:
- - colorama # [linux] - - importlib-metadata # [py <= 38] + - colorama >=0.4.0 # [linux] + - importlib-metadata >=4.0.0 # [py <= 38]Please verify these minimum versions against the package's requirements.
Also applies to: 39-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/el_gato/build.sh (0 hunks)
- recipes/el_gato/meta.yaml (3 hunks)
💤 Files with no reviewable changes (1)
- recipes/el_gato/build.sh
🧰 Additional context used
🔇 Additional comments (4)
recipes/el_gato/meta.yaml (4)
2-3
: Version and hash update looks good.The version has been correctly updated to 1.20.0 as per the PR objective, and the hash has been updated accordingly.
49-53
: About section updates look good.The additions to the about section (license family, dev URL, and doc URL) provide more comprehensive metadata for the package. The minor stylistic change to the summary (adding quotes) is also fine.
Line range hint
1-53
: Overall changes align well with PR objectives.The updates to the meta.yaml file successfully accomplish the PR objective of updating el_gato to version 1.20.0. The changes include:
- Version and hash update
- New build script replacing the deleted build.sh file
- Adjusted run_exports pinning
- Updated requirements
- Enhanced about section metadata
These changes comprehensively address the version update and should resolve the build issues mentioned in the PR objectives.
To ensure all changes are consistent, please run the following script to check for any remaining references to the old version:
#!/bin/bash # Description: Check for any remaining references to the old version # Test: Search for the old version number rg '1\.19\.0' --type yaml --type python
22-22
: Run exports pinning change looks good, but verify impact.The change from
max_pin='x.x'
tomax_pin='x'
allows for more flexible version compatibility. This means the package will be compatible with any future versions that share the same major version number.Please verify that this change aligns with the package's versioning strategy and doesn't introduce any compatibility issues. Run the following script to check for any explicit version pins in the package code:
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 (2)
recipes/el_gato/meta.yaml (2)
17-20
: LGTM! Consider adding a comment for clarity.The new build script effectively replaces the deleted build.sh file. It ensures that the database files are properly installed with the package.
Consider adding a brief comment explaining the purpose of the
--no-deps --no-build-isolation --no-cache-dir
flags in the pip install command for better maintainability.
51-51
: LGTM! Consider updating the doc_url.The changes in the 'about' section improve consistency and provide more comprehensive metadata. The use of template variables in dev_url and doc_url is good for maintainability.
Consider updating the doc_url to use the
{{ user }}
variable for consistency:doc_url: "https://github.com/{{ user }}/{{ name }}/blob/{{ version }}/README.md"Also applies to: 53-53, 55-57
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/el_gato/meta.yaml (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
recipes/el_gato/meta.yaml (4)
2-3
: Verify the correct version number.The version has been updated to "1.20.1", which is different from the version mentioned in the PR title (1.20.0). Please confirm if this is the intended version.
The hash update is consistent with the version change.
22-22
: LGTM! Improved version compatibility.The update to
max_pin='x'
allows for more flexible version compatibility, which is consistent with the package update and may help with dependency management.
26-26
: LGTM! Verify new dependencies if needed.The addition of Python as a build requirement and the new run requirements (colorama for Linux and importlib-metadata for Python <= 3.8) appear appropriate for the updated package version.
Please verify that these new dependencies are indeed necessary for the 1.20.1 version of el_gato.
Also applies to: 39-40
42-44
: LGTM! Good documentation of lint skip reason.The addition of the 'extra' section with the skip-lints entry clearly explains why noarch: python is not used. This improves the maintainability of the package by documenting the reason for a potential lint warning.
Update el_gato to 1.20.0
Replaces #51051. Hopefully fixed the build issues experienced in that PR.