Skip to content

Comments

Check final status for workflow jobs#23

Open
Goooler wants to merge 11 commits intosozercan:mainfrom
Goooler:ci-checks
Open

Check final status for workflow jobs#23
Goooler wants to merge 11 commits intosozercan:mainfrom
Goooler:ci-checks

Conversation

@Goooler
Copy link
Contributor

@Goooler Goooler commented Jan 3, 2026

Also groups all required workflow jobs into the ci.yml file.

Benefits for

You can enable the check in the repo settings, this enables auto-merge for PRs.

Also groups all required workflow jobs into the ci.yml file.

You can enable the check in repo settings, this unblocks auto-merge for PRs.
Copilot AI review requested due to automatic review settings January 3, 2026 00:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates all CI workflow jobs into a single ci.yml file and introduces a final-status check job that can be used as a required status check in branch protection rules. This simplification enables auto-merge functionality for pull requests.

Key changes:

  • Merges three separate workflow files (tests.yml, lint.yml, dev-build.yml) into one unified ci.yml
  • Adds a final-status job that aggregates the results of all required jobs for branch protection
  • Removes path-specific filtering to trigger CI on all pull requests and main branch pushes

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/tests.yml Removed - unit and UI test jobs migrated to ci.yml
.github/workflows/lint.yml Removed - SwiftLint and SwiftFormat jobs migrated to ci.yml
.github/workflows/dev-build.yml Removed - build and DMG creation job migrated to ci.yml
.github/workflows/ci.yml New consolidated workflow containing all jobs plus final-status check

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- name: Check
run: |
results=$(tr -d '\n' <<< '${{ toJSON(needs.*.result) }}')
if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in this condition is inverted and will not correctly detect job failures. The command grep -q -v -E '(failure|cancelled)' with the -v flag matches lines that do NOT contain "failure" or "cancelled". When negated with !, the condition only fails when ALL results are failures or cancellations, not when ANY result fails.

This means if you have a mix of success and failure (e.g., ["success","failure","success"]), the check will incorrectly pass because the grep will find "success" entries that don't match the pattern.

The condition should check if the results contain any failure or cancelled status and fail in that case. Consider using a logic like: grep -q -E '(failure|cancelled)' <<< "$results" (without the -v and !) to fail when any job has failed or been cancelled.

Suggested change
if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
if grep -q -E '(failure|cancelled)' <<< "$results"; then

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Goooler
Copy link
Contributor Author

Goooler commented Jan 3, 2026

The unit test failures are unrelated, may fix them first and rebase this PR.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- name: Check
run: |
results=$(tr -d '\n' <<< '${{ toJSON(needs.*.result) }}')
if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The grep logic appears to be inverted. The current condition '! grep -q -v -E' will fail if there are NO failures/cancellations (all success), which is the opposite of intended behavior. The logic should check if any job failed or was cancelled, and exit 1 in that case. Consider using: 'if grep -q -E "(failure|cancelled)" <<< "$results"; then'

Suggested change
if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
if grep -q -E '(failure|cancelled)' <<< "$results"; then

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goooler
Copy link
Contributor Author

Goooler commented Jan 3, 2026

It works fine, and the real diff is:

`diff dev-build.yml ci.yml`

1c1
< name: Dev Build
---
> name: CI
9,15c9,10
<     branches: [main]
<     paths:
<       - "**.swift"
<       - "**.xcodeproj/**"
<       - "**.xcworkspace/**"
<       - "Assets.xcassets/**"
<       - ".github/workflows/dev-build.yml"
---
>     branches:
>       - main
17,22d11
<     paths:
<       - "**.swift"
<       - "**.xcodeproj/**"
<       - "**.xcworkspace/**"
<       - "Assets.xcassets/**"
<       - ".github/workflows/dev-build.yml"
26c15,16
<   build:
---
>   swiftlint:
>     name: SwiftLint
28c18,24
<     timeout-minutes: 30
---
>     timeout-minutes: 10
>     steps:
>       - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
>       - run: brew install swiftlint
>       - name: Run SwiftLint
>         run: |
>           swiftlint lint --strict --reporter github-actions-logging
29a26,29
>   swiftformat:
>     name: SwiftFormat
>     runs-on: macos-26
>     timeout-minutes: 10
31,32c31,38
<       - name: Checkout code
<         uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
---
>       - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
>       - run: brew install swiftformat
>       - name: Check formatting
>         run: |
>           swiftformat --lint . 2>&1 || {
>             echo "::error::Code is not formatted. Run 'swiftformat .' locally and commit the changes."
>             exit 1
>           }
33a40,45
>   macos_unit_tests:
>     name: macOS Unit Tests
>     timeout-minutes: 20
>     runs-on: macos-26
>     steps:
>       - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
35a48,69
>       - name: Show Xcode version
>         run: xcodebuild -version
>       - name: Run unit tests
>         run: |
>           set -o pipefail
>           xcodebuild \
>             -project Kaset.xcodeproj \
>             -scheme Kaset \
>             -destination 'platform=macOS' \
>             -derivedDataPath ./build \
>             -only-testing:KasetTests \
>             CODE_SIGN_IDENTITY="" \
>             CODE_SIGNING_REQUIRED=NO \
>             CODE_SIGNING_ALLOWED=NO \
>             test
>       - name: Upload test results on failure
>         if: failure()
>         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v4
>         with:
>           name: macOS-TestResults
>           path: ./build/*.xcresult
>           retention-days: 7
36a71,78
>   macos_ui_tests:
>     name: macOS UI Tests
>     timeout-minutes: 30
>     runs-on: macos-26
>     steps:
>       - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
>       - name: Select Xcode version
>         run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/Developer
38a81,100
>       - name: Run UI tests
>         run: |
>           set -o pipefail
>           xcodebuild \
>             -project Kaset.xcodeproj \
>             -scheme Kaset \
>             -destination 'platform=macOS' \
>             -derivedDataPath ./build \
>             -only-testing:KasetUITests \
>             CODE_SIGN_IDENTITY="" \
>             CODE_SIGNING_REQUIRED=NO \
>             CODE_SIGNING_ALLOWED=NO \
>             test
>       - name: Upload test results on failure
>         if: failure()
>         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v4
>         with:
>           name: macOS-UITestResults
>           path: ./build/*.xcresult
>           retention-days: 7
39a102,111
>   build:
>     name: Build
>     runs-on: macos-26
>     timeout-minutes: 30
>     steps:
>       - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
>       - name: Select Xcode version
>         run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/Developer
>       - name: Show Xcode version
>         run: xcodebuild -version
43d114
< 
58d128
< 
90d159
< 
97a167,186
> 
>   # Status check that is required in branch protection rules.
>   final-status:
>     needs:
>       - swiftlint
>       - swiftformat
>       - macos_unit_tests
>       - macos_ui_tests
>       - build
>     runs-on: ubuntu-latest
>     if: always()
>     steps:
>       - name: Check
>         run: |
>           results=$(tr -d '\n' <<< '${{ toJSON(needs.*.result) }}')
>           if ! grep -q -v -E '(failure|cancelled)' <<< "$results"; then
>             echo "One or more required jobs failed"
>             exit 1
>           fi
>           echo "All required jobs completed successfully."

@Goooler Goooler changed the title Add a final-status check on CI Check final-status for workflow jobs Jan 3, 2026
@Goooler Goooler changed the title Check final-status for workflow jobs Check final status for workflow jobs Jan 3, 2026
pull_request:
workflow_dispatch:

jobs:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like this but its removing filtering based on files so a change in readme doesn't trigger a full ci run

can we use something like https://github.com/dorny/paths-filter to add a filter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intended. Removing all filters ensures all file changes will trigger the workflow.

runs-on: macos-26
steps:
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v4
- uses: actions/cache@v5
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure to pin to sha for all actions

Copy link
Contributor Author

@Goooler Goooler Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted! Will land it in a separate PR.

f03c4be

# Conflicts:
#	.github/workflows/dev-build.yml
#	.github/workflows/lint.yml
#	.github/workflows/tests.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants