Conda build maintenance #188
Conversation
|
Warning Rate limit exceeded@fabianegli has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request modifies the GitHub Actions workflow file for Conda build, introducing a Changes
Poem
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 (
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨Explore these optional code suggestions:
|
![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
🧹 Nitpick comments (1)
.github/workflows/conda-build.yml (1)
23-58: Consider optimizing environment activations.The workflow activates the conda environment multiple times. Consider consolidating steps or using a composite action to reduce repetition.
Example approach:
- name: Build and test run: | source "$CONDA/etc/profile.d/conda.sh" conda activate sdrf-pipelines # Install build tools conda install conda-build anaconda-client # Build and install conda build --package-format .tar.bz2 recipe PACKAGE_PATH=$(find /usr/share/miniconda/envs/sdrf-pipelines/conda-bld/noarch/ -name "sdrf-pipelines-*.tar.bz2" -type f -print -quit) conda install --offline "$PACKAGE_PATH" # Run tests parse_sdrf --help parse_sdrf validate-sdrf --sdrf_file tests/data/reference/PDC000126/PDC000126.sdrf.tsv parse_sdrf validate-sdrf --sdrf_file tests/data/reference/PDC000126/PDC000126.sdrf.tsv --use_ols_cache_only🧰 Tools
🪛 actionlint (1.7.4)
27-27: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
38-38: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/conda-build.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/conda-build.yml
27-27: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
32-32: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
38-38: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
.github/workflows/conda-build.yml (3)
8-10: LGTM! Good practice using login shell.Setting
bash -l {0}as the default shell ensures proper conda initialization and environment activation across different runner environments.
13-22: LGTM! Good configuration of Miniconda setup.Using latest action versions and explicit Python version specification with appropriate channel configuration.
45-58: LGTM! Comprehensive test coverage.Good test strategy:
- Basic functionality check with --help
- Validation with real data
- Cache-only validation test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/conda-build.yml (2)
26-28: Consider pinning dependency versions.While the essential build tools are included, consider pinning their versions for better reproducibility.
- conda install conda-build conda-verify anaconda-client + conda install conda-build=3.27.0 conda-verify=3.3.1 anaconda-client=1.12.0
36-38: Consider adding more comprehensive package tests.The current test only verifies that the command exists. Consider adding tests for basic functionality.
- parse_sdrf --help + # Basic command existence test + parse_sdrf --help + # Version check + parse_sdrf --version + # Test basic parsing functionality + parse_sdrf convert-to-json --help
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/conda-build.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
.github/workflows/conda-build.yml (4)
8-10: LGTM! Good shell configuration.Setting
bash -l {0}as the default shell ensures proper conda initialization across all steps.
30-34: LGTM! Well-structured build process.Good practices implemented:
- Using .tar.bz2 format for better compatibility
- Capturing package path efficiently
- Installing package offline
40-46: LGTM! Verify test data availability.The validation tests are well-structured with both online and offline cache scenarios.
Let's verify the test data exists:
✅ Verification successful
Test data verification successful
The referenced SDRF test file exists at the specified location and is accessible for the validation tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify test data existence if [ -f "tests/data/reference/PDC000126/PDC000126.sdrf.tsv" ]; then echo "Test data exists" else echo "Test data not found" fiLength of output: 105
16-24: LGTM! Verify environment file existence.The Miniconda setup is well-configured with explicit versions and correct channel order.
Let's verify the referenced environment file exists:
✅ Verification successful
Environment file verification successful ✓
The environment.yml file exists and its configuration aligns perfectly with the workflow settings, including matching environment name, correct channel order, and Python 3.10 specification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify environment.yml exists and contains valid conda configuration if [ -f "environment.yml" ]; then echo "environment.yml exists" cat environment.yml else echo "environment.yml not found" fiLength of output: 393
|
@ypriverol Feel free to merge if you think it's good. |
User description
This PR fixes the conda package build and includes some code maintenance.
PR Type
Enhancement, Configuration changes
Description
Added default shell configuration for workflow steps.
Specified package output format in conda build.
Improved indentation for better readability and maintenance.
Changes walkthrough 📝
conda-build.yml
Enhanced conda build workflow configuration.github/workflows/conda-build.yml
defaultssection to set a default shell..tar.bz2as the package output format.Summary by CodeRabbit
.tar.bz2package format