Filter CCR jobs in update-required-checks.sh
#3275
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the script that manages required status checks to exclude additional CI/CD job names from the required checks list. The change adds four more job names to the filter: "Agent", "Cleanup artifacts", "Prepare", and "Upload results".
- Expands the filter criteria in the jq command to exclude four new job names from required checks
|
|
||
| # Ignore any checks with "https://", CodeQL, LGTM, Update, and ESLint checks. | ||
| CHECKS="$(gh api repos/github/codeql-action/commits/"${GITHUB_SHA}"/check-runs --paginate | jq --slurp --compact-output --raw-output '[.[].check_runs.[] | select(.conclusion != "skipped") | .name | select(contains("https://") or . == "CodeQL" or . == "Dependabot" or . == "check-expected-release-files" or contains("Update") or contains("ESLint") or contains("update") or contains("test-setup-python-scripts") | not)] | unique | sort')" | ||
| CHECKS="$(gh api repos/github/codeql-action/commits/"${GITHUB_SHA}"/check-runs --paginate | jq --slurp --compact-output --raw-output '[.[].check_runs.[] | select(.conclusion != "skipped") | .name | select(contains("https://") or . == "CodeQL" or . == "Dependabot" or . == "check-expected-release-files" or contains("Update") or contains("ESLint") or contains("update") or contains("test-setup-python-scripts") or . == "Agent" or . == "Cleanup artifacts" or . == "Prepare" or . == "Upload results" | not)] | unique | sort')" |
There was a problem hiding this comment.
[nitpick] The jq filter has become very long and difficult to maintain with multiple exact matches mixed with substring matches. Consider refactoring this into a more maintainable pattern, such as defining an array of patterns to exclude or breaking the complex filter into multiple steps for clarity.
See below for a potential fix:
# Define exclusion patterns for check names (exact matches and substrings)
EXCLUDE_PATTERNS=('https://' 'CodeQL' 'Dependabot' 'check-expected-release-files' 'Update' 'ESLint' 'update' 'test-setup-python-scripts' 'Agent' 'Cleanup artifacts' 'Prepare' 'Upload results')
# Convert bash array to jq array
EXCLUDE_PATTERNS_JQ=$(printf '"%s",' "${EXCLUDE_PATTERNS[@]}" | sed 's/,$//')
# Use jq to filter out checks matching any exclusion pattern
CHECKS="$(gh api repos/github/codeql-action/commits/"${GITHUB_SHA}"/check-runs --paginate | jq --slurp --compact-output --raw-output --argjson exclude_patterns "[$EXCLUDE_PATTERNS_JQ]" '
[.[].check_runs.[]
| select(.conclusion != "skipped")
| .name
| select(
[
$exclude_patterns[]
| (. as $pat
| if ($pat | test("^https://"))
then (. | contains($pat))
else (. == $pat or . | contains($pat))
end
)
]
| any
) | not
)
] | unique | sort
')"
esbena
left a comment
There was a problem hiding this comment.
Agreed. That one liner is a scary one. According to git blame, it has always been like that FWIW.
For reference, if we use the API a bit more, it is possible to tease out this run: https://github.com/github/codeql-action/actions/runs/19083414184/job/54517793831 which has a more robust name (if we must use names): "Copilot code review". Incidentally, it also has the full list of names we should check for in the current implementation.
I think we did something similar recently on an internal repository.
I ran the script for #3274 and it added the CCR jobs. This PR modifies it to filter those out.
I ran the script with these changes and it seemed to work OK.
I am not particularly keen on the filtering based on names here and the long
jqcommand. Maybe we could look at improving the script to make it a bit nicer.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
CI
How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
N/A
How will you know if something goes wrong after this change is released?
N/A
Merge / deployment checklist