ci: Switch PR checks for linux-64 and osx-64 to GitHub Actions #51720
Conversation
|
A couple of things that I think are happening because these changes are on a branch and not master:
Not sure how else to test this besides just merging. |
📝 WalkthroughWalkthroughThe pull request introduces several modifications to the GitHub Actions workflows and Azure pipelines. In 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
azure-pipeline-master.yml (1)
Line range hint
1-99: Consider documenting the Azure to GitHub Actions transition strategyThe approach to disable Azure pipelines while maintaining the configuration and fallback builds is sound. However, consider:
- Adding comments in the file header explaining the transition status
- Creating a timeline for removing this file once the transition is complete
- Documenting the fallback strategy for existing PRs in the repository documentation
This will help future maintainers understand the transition process and cleanup requirements.
azure-pipeline.yml (1)
Pipeline configuration should be cleaned up and documented
The search results confirm that there are two Azure pipeline files (
azure-pipeline.ymlandazure-pipeline-master.yml), both marked as disabled in favor of GitHub Actions. The README.md still shows Azure DevOps badges for some builds, but they appear to be specifically for nightly builds.Given this context:
- The pipeline should be properly documented to explain its current status and potential future use
- Consider archiving these files to a
.archivedirectory since they're disabled but kept for reference- Update the README.md to remove or update the Azure DevOps badges if they're no longer relevant
🔗 Analysis chain
Line range hint
1-360: Consider cleanup of unused pipeline configuration.While keeping the full pipeline configuration might be intentional for fallback purposes as mentioned in the PR objectives, consider:
- Adding a comment explaining why the pipeline stages are kept (for fallback/reference)
- Moving this configuration to a separate reference file if it's only kept for documentation
- If the stages are needed for fallback, add clear documentation about when and how they're triggered
This will help future maintainers understand the purpose of this seemingly inactive pipeline configuration.
Let's verify if this pipeline is referenced elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to this pipeline in other files rg -l "azure-pipeline.yml"Length of output: 26
Script:
#!/bin/bash # Let's try searching for any Azure Pipelines related configuration or documentation rg -i "azure" -g "*.{md,yml,yaml}" -A 3 # Also check for any GitHub Actions workflow files that might reference Azure fd -e yml -e yaml -p ".github/workflows"Length of output: 10865
🧰 Tools
🪛 yamllint
[error] 5-5: trailing spaces
(trailing-spaces)
.github/workflows/master.yml (1)
105-169: Consider moving commented job to a separate file.While the commented-out OSX-ARM64 job is well-documented and the disabling aligns with the PR objectives (moving to CircleCI due to runner limits), keeping large commented blocks in the workflow file can make maintenance more challenging.
Consider moving this configuration to a separate file (e.g.,
.github/workflows/osx-arm64.yml.disabledor.github/workflows/templates/osx-arm64.yml) for potential future reuse while keeping the main workflow file clean..github/workflows/PR.yml (1)
Line range hint
1-300: Well-structured CI transition planThe workflow changes demonstrate a well-thought-out approach to transitioning CI checks:
- Proper sharing of common functions across jobs
- Smart optimization of limited OSX runners
- Clear handling of platform-specific requirements
- Good separation of concerns between different CI platforms
Consider documenting these CI platform responsibilities in the repository's contributing guidelines to help future contributors understand where different builds run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- .github/workflows/PR.yml (5 hunks)
- .github/workflows/master.yml (2 hunks)
- azure-pipeline-master.yml (2 hunks)
- azure-pipeline.yml (1 hunks)
🧰 Additional context used
🪛 yamllint
azure-pipeline.yml
[error] 5-5: trailing spaces
(trailing-spaces)
🔇 Additional comments (14)
azure-pipeline-master.yml (4)
1-5: LGTM: Pipeline triggers correctly disabledThe changes align with the PR objective to disable Azure pipelines due to memory issues. The commented master trigger provides useful context for future reference.
41-44: Verify git range calculation for merged PRsThe git range calculation uses
$(Build.SourceVersion)~1 $(Build.SourceVersion), which assumes the merge commit is available. Ensure this works correctly with the new GitHub Actions setup.#!/bin/bash # Check how git range is calculated in GitHub Actions workflows rg -A 2 'git.*range' .github/workflows/
Line range hint
89-93: Verify OSX-specific configurations in GitHub ActionsThe OSX job sources MACOSX variables and uses the fallback build option. Ensure these configurations are properly mirrored in the new GitHub Actions workflows.
✅ Verification successful
OSX configurations are properly mirrored in GitHub Actions
The verification shows that the GitHub Actions workflows have properly implemented the OSX-specific configurations:
- macOS runners are configured using
macos-13in multiple workflows (master.yml, PR.yml, Bulk.yml)common.shis properly sourced in the workflows, which sets up the OSX SDK viarun_conda_forge_build_setup- The fallback build option is present in the master workflow with
--fallback buildparameter🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if GitHub Actions workflows handle OSX-specific configurations echo "Checking for MACOSX variable handling:" rg -A 2 'MACOSX|common.sh' .github/workflows/ echo "Checking for fallback build option:" rg -A 2 'fallback build' .github/workflows/Length of output: 1897
Script:
#!/bin/bash # Let's check for OSX/macOS specific configurations and runners echo "Checking for macOS runners and configurations:" rg -A 5 'runs-on:.*macos' .github/workflows/ echo "Checking for common.sh sourcing:" rg -A 5 'source.*common.sh' .github/workflows/ echo "Checking entire common.sh file if it exists:" fd common.sh --exec cat {}Length of output: 4243
Line range hint
17-18: Review the master branch checkout stepThe job explicitly checks out the master branch, which might interfere with PR validation. Consider removing this step since we're disabling Azure pipelines.
✅ Verification successful
The previous search was limited to .github/workflows. Let's expand our search to the entire repository and also look for any CI configuration files to better understand the context.
Based on the search results, I can see that
git checkout masterappears in bothazure-pipeline-master.ymlandazure-pipeline-nightly.yml. Since we're disabling Azure pipelines, I can generate the final response:Remove the master branch checkout step as it's no longer needed
The
git checkout mastercommand inazure-pipeline-master.ymlcan be safely removed since Azure pipelines are being disabled and this step is not needed anymore.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if other workflow files also perform master branch checkout rg -l 'git checkout master' .github/workflows/Length of output: 48
Script:
#!/bin/bash # Check for any git checkout master commands in the entire repository rg -l "git checkout master" # Also check for any variations of master branch checkout rg -l "checkout\s+master" # Look for any GitHub Actions or Azure Pipeline configurations fd -e yml -e yamlLength of output: 357320
azure-pipeline.yml (2)
1-4: LGTM! Clear documentation of the change reason.The comment clearly explains why Azure checks are being disabled, which is helpful for future maintenance.
5-5:⚠️ Potential issueFix trailing whitespace.
Remove trailing spaces after
trigger: none.Apply this diff:
-trigger: none +trigger: noneLikely invalid or redundant comment.
🧰 Tools
🪛 yamllint
[error] 5-5: trailing spaces
(trailing-spaces)
.github/workflows/master.yml (3)
99-102: LGTM! Command changes are consistent.The changes mirror the Linux build modifications, maintaining consistency across platforms. The OSX-specific setup step is correctly placed before the build command.
Line range hint
1-169: Verify workflow job dependencies and concurrency.The workflow changes look good overall. To ensure smooth operation after the transition:
Let's verify the workflow structure and concurrency settings:
#!/bin/bash # Description: Check for potential workflow issues # Test 1: Check for job dependencies rg --type yaml 'needs:' .github/workflows/ # Test 2: Check concurrency settings rg --type yaml 'concurrency:' .github/workflows/ # Test 3: Verify consistent max-parallel settings rg --type yaml 'max-parallel:' .github/workflows/
47-52: LGTM! Verify git range parameter handling.The command changes align well with the transition strategy. The fallback option ensures compatibility with existing PRs, and the artifact source parameter helps track the build origin.
Let's verify the git range parameter handling:
✅ Verification successful
Git range parameter is correctly implemented
The git range parameter usage is consistent with the workflow's purpose:
- PR workflow uses
origin/$GITHUB_BASE_REF HEADto compare changes against the base branch- Master workflow uses
${GITHUB_SHA}~1 ${GITHUB_SHA}to compare the merged commit with its parentThis difference is intentional and correct:
- For PRs, we need to check changes against the base branch
- For merged commits, we need to process just the latest merge
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if git range syntax is consistent across workflows # Expect: Same git range syntax in PR and master workflows # Search for git range parameter usage rg --type yaml 'git-range.*GITHUB_SHA' .github/workflows/Length of output: 950
Script:
#!/bin/bash # Let's try again with the correct pattern to search for git-range usage rg --type yaml 'git-range.*GITHUB_SHA' .github/workflows/ -A 2 -B 2Length of output: 1242
Script:
#!/bin/bash # Let's check PR workflow to compare git range handling rg --type yaml 'bioconda-utils' .github/workflows/PR.yml -A 5 -B 2Length of output: 2514
.github/workflows/PR.yml (5)
42-42: LGTM: Common shell functions integrationThe addition of
source common.shis well-placed and consistent with the workflow's setup phase.
153-153: LGTM: Smart optimization for OSX buildsAdding the dependency on
build-linuxis a good optimization that prevents wasting limited OSX runners on builds that would fail anyway.
183-185: LGTM: Proper OSX SDK setupThe addition of
run_conda_forge_build_setupensures proper SDK configuration for OSX builds.
219-300: Verify arm64 builds on CircleCIThe disabling of arm64 builds on GitHub Actions is well-documented and aligns with the PR objectives. However, we should verify that these builds are properly configured on CircleCI.
Let's verify the CircleCI configuration:
#!/bin/bash # Check CircleCI configuration for arm64 builds echo "Checking CircleCI config for arm64 builds:" if [ -f .circleci/config.yml ]; then grep -A 10 "osx-arm64" .circleci/config.yml else echo "CircleCI config not found in expected location" fi
97-102: Verify Docker image handling after command removalsWhile the addition of common shell functions is good, we should verify that Docker image handling is still properly managed after the removal of explicit Docker commands.
Let's verify if Docker image handling is managed elsewhere:
|
Turns out I also had to edit the master branch rules to require the GitHub Actions checks to pass instead of the Azure checks. |
Due to the memory issues on Azure lately, use GitHub Actions for linux-64 and osx-64 builds. Existing PRs with Azure builds should be caught by the fallback option when merged to master. The nightly builds will remain on Azure for now.