-
-
Notifications
You must be signed in to change notification settings - Fork 363
chore(ci): Add required checks job for lint clang formatting #6004
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
chore(ci): Add required checks job for lint clang formatting #6004
Conversation
Co-authored-by: phil.niedertscheider <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6004 +/- ##
=============================================
- Coverage 86.723% 86.712% -0.011%
=============================================
Files 424 424
Lines 36712 36712
Branches 17359 17361 +2
=============================================
- Hits 31838 31834 -4
- Misses 4828 4831 +3
- Partials 46 47 +1 see 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
LGTM
…ang-formatting-3c58
…ang-formatting-3c58
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.
Bug: Clang Formatting Lint Runs On All Pushes
The lint
job now runs for all push
events to main
, removing the previous path-based filtering. This causes unnecessary CI runs when commits don't modify files relevant to clang formatting, impacting CI efficiency.
.github/workflows/lint-clang-formatting.yml#L40-L41
sentry-cocoa/.github/workflows/lint-clang-formatting.yml
Lines 40 to 41 in 6923f4e
# Run the job only for PRs with related changes or non-PR events. | |
if: github.event_name != 'pull_request' || needs.files-changed.outputs.run_lint_clang_formatting_for_prs == 'true' |
.github/workflows/lint-clang-formatting.yml#L3-L6
sentry-cocoa/.github/workflows/lint-clang-formatting.yml
Lines 3 to 6 in 6923f4e
on: | |
push: | |
branches: | |
- main |
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a9fac2e | 1212.45 ms | 1219.67 ms | 7.22 ms |
ebc72be | 1221.24 ms | 1249.66 ms | 28.42 ms |
daeb716 | 1215.41 ms | 1246.52 ms | 31.11 ms |
2b02431 | 1229.63 ms | 1248.98 ms | 19.35 ms |
326984b | 1235.06 ms | 1252.75 ms | 17.69 ms |
d637379 | 1226.43 ms | 1250.77 ms | 24.34 ms |
ad964ca | 1234.73 ms | 1254.88 ms | 20.15 ms |
5bf2b17 | 1213.53 ms | 1238.54 ms | 25.01 ms |
891fd1d | 1220.02 ms | 1227.60 ms | 7.57 ms |
67e8e3e | 1220.08 ms | 1229.23 ms | 9.15 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a9fac2e | 23.75 KiB | 879.53 KiB | 855.78 KiB |
ebc72be | 23.75 KiB | 908.22 KiB | 884.47 KiB |
daeb716 | 23.75 KiB | 928.16 KiB | 904.41 KiB |
2b02431 | 23.75 KiB | 850.73 KiB | 826.98 KiB |
326984b | 23.74 KiB | 926.64 KiB | 902.90 KiB |
d637379 | 23.75 KiB | 855.38 KiB | 831.63 KiB |
ad964ca | 23.75 KiB | 913.17 KiB | 889.42 KiB |
5bf2b17 | 23.75 KiB | 913.27 KiB | 889.52 KiB |
891fd1d | 23.75 KiB | 919.92 KiB | 896.17 KiB |
67e8e3e | 23.75 KiB | 919.91 KiB | 896.16 KiB |
📜 Description
Migrates the
lint-clang-formatting.yml
workflow to usedorny/paths-filter
for conditional execution on pull requests and introduces alint_clang_formatting-required-check
job to enforce required checks.💡 Motivation and Context
This change is part of the ongoing effort to transition workflows from
on.[event].paths
filtering tofiles-changed
jobs usingdorny/paths-filter
(see #5951). The aim is to make CI checks more efficient by running them conditionally based on file changes, while ensuring a reliable required check for branch protection rules. This implementation follows the pattern established in #5893 fortests.yml
andapi-stability.yml
.💚 How did you test it?
This is a CI configuration change. The functionality will be verified by observing the workflow's execution on a pull request with relevant file changes and one without.
Fixes #5958
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.#skip-changelog