-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ci: Let GH CI skip build targets specified in GH CI settings #6597
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the CI/CD workflow by introducing conditional checks for job execution. Each job now evaluates one or more environment variables before running, which allows skipping jobs based on variable presence. For example, jobs related to building dependencies or source for various environments check if specific 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
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
1-210
: Overall CI configuration review.
The modifications introduce clear conditional checks across various jobs usingif
conditions based onSKIP_*
variables. This design allows users to selectively skip build targets, addressing the CI flexibility limitation described in the PR objectives. As a next step, consider updating the CI documentation (or the README) to briefly describe the purpose of these environment variables and how to use them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
(12 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
🔇 Additional comments (12)
.github/workflows/build.yml (12)
23-23
: Validation of arm‑linux job skip condition.
The new conditionif: ${{ vars.SKIP_ARM_LINUX == '' }}
ensures that the arm‑linux dependency job runs only when the
SKIP_ARM_LINUX
variable is empty (i.e. not explicitly set to skip). This aligns well with the PR objective of letting users opt out of specific build targets.
32-36
: Review of linux64 dependency job multi‑flag condition.
The multi‑line condition:if: | vars.SKIP_LINUX64 == '' || vars.SKIP_LINUX64_FUZZ == '' || vars.SKIP_LINUX64_SQLITE == '' || vars.SKIP_LINUX64_UBSAN == ''
ensures that the job runs if any one of these flags is unset. This approach effectively provides granular control over skipping specific build targets.
45-47
: Assessment of multiprocess dependency job condition.
The condition:if: | vars.SKIP_LINUX64_MULTIPROCESS == '' || vars.SKIP_LINUX64_TSAN == ''
ensures that the
depends-linux64_multiprocess
job is executed if either the multiprocess build or the TSAN build is not skipped. Please verify that using the OR operator here fully reflects the intended behavior for shared dependencies between these two build targets.
56-56
: Validation of nowallet dependency job skip condition.
The conditionif: ${{ vars.SKIP_LINUX64_NOWALLET == '' }}
is straightforward and correctly ensures that the job executes only when the corresponding skip flag is not set.
65-65
: Validation of mac dependency job skip condition.
The conditionif: ${{ vars.SKIP_MAC == '' }}
ensures that the mac dependency job is run only when the
SKIP_MAC
variable is unset. This is consistent with the approach taken for other platforms.
74-74
: Validation of win64 dependency job skip condition.
The conditionif: ${{ vars.SKIP_WIN64 == '' }}
correctly restricts the win64 job to run only when no skip flag is provided, which is in line with the desired behavior.
92-92
: Review of linux64 source build skip condition.
The conditionif: ${{ vars.SKIP_LINUX64 == '' }}
ensures that the linux64 source build job is executed only when the
SKIP_LINUX64
flag is empty, maintaining consistency with the dependency jobs.
102-102
: Review of linux64_fuzz source build condition.
The conditionif: ${{ vars.SKIP_LINUX64_FUZZ == '' }}
appropriately gatekeeps the execution of the fuzz build job, matching the established pattern used in this workflow.
112-112
: Review of linux64_multiprocess source build condition.
The conditionif: ${{ vars.SKIP_LINUX64_MULTIPROCESS == '' }}
ensures that the multiprocess source build runs only when it isn’t explicitly skipped. This is clear and consistent with your overall CI strategy.
131-131
: Review of linux64_sqlite source build condition.
The conditionif: ${{ vars.SKIP_LINUX64_SQLITE == '' }}
is implemented correctly, so the sqlite build is executed only when the corresponding skip flag is not set.
141-141
: Review of linux64_tsan source build condition.
The conditionif: ${{ vars.SKIP_LINUX64_TSAN == '' }}
ensures that the TSAN build job runs only when the
SKIP_LINUX64_TSAN
variable is empty. This clear gating mechanism matches the intended behavior.
151-151
: Review of linux64_ubsan source build condition.
The conditionif: ${{ vars.SKIP_LINUX64_UBSAN == '' }}
maintains consistency with the other target-specific conditions, ensuring that the UBSAN build is executed only when not skipped.
Issue being fixed or feature implemented
Unfortunately
if
on Github can't handle regex so a solution implemented for Gitlab (#6591) can't be used as easily here and requires implementing pretty large workarounds, see #6591 (comment). This PR implements an alternative approach where instead of specifying which targets you want to build you'd instead specify targets you don't need, one by one.What was done?
How Has This Been Tested?
https://github.com/UdjinM6/dash/actions/runs/13532112313
Breaking Changes
Checklist: