Skip to content

Commit 8d9b69e

Browse files
authored
ci: fix shellcheck findings and trim workflow comments (#1967) (#2023)
Step 5, part 1 of #1967, over every workflow except unit-tests.yml and experimental.yml (open PRs edit those): - Fix actionlint's shellcheck findings at the root: quote the `.[extras]` pip targets (SC2102), group `$GITHUB_OUTPUT` writes into one quoted redirect (SC2129, SC2086), quote `exit "$status"` (SC2086), and run isort through `find ... -exec ... {} +` instead of an unquoted `$(find)` (SC2046). - Trim comments: drop restatements of the YAML, history narration, duplicated rationale, CodeQL template boilerplate and dead commented-out keys; keep each non-obvious why and its issue ref. - Cite create-release.yml, python-compatibility.yml and coverage-comment.yml by job and step name in releasing.rst and workflows.rst (decision A), and replace the line-anchor test with one that rejects line-number citations of those files. - Update the conda pin test for the quoted `'.[all]'`. actionlint 1.7.12 now reports nothing outside unit-tests.yml.
1 parent 609bd93 commit 8d9b69e

16 files changed

Lines changed: 795 additions & 1560 deletions

‎.github/workflows/ci-timing.yml‎

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,19 @@
11
name: CI Timing
22

3-
# The periodic CI-duration review #1538 asked for. util/ci_timing.py reads the
4-
# last week of Unit Tests, Compat, Lint, CodeQL, Pages and Experimental Tests
5-
# runs through the API, writes a report to the job summary and to an artifact,
6-
# and fails the run when a
7-
# HARD threshold fires (status 1; 2 and 3 are an unreadable API and a crash, each
8-
# annotated as such). GitHub emails a scheduled workflow's failures to whoever
9-
# last edited its cron line, so the owner hears about it with no `issues: write`.
10-
# docs/source/contributing/workflows.rst has the thresholds and why each sits
11-
# where it does; conventions/process.rst has what to do when one fires.
3+
# The periodic CI-duration review (#1538). util/ci_timing.py reads the last
4+
# week of runs through the API, writes a report to the job summary and an
5+
# artifact, and fails when a HARD threshold fires (status 1; 2 and 3 are an
6+
# unreadable API and a crash). GitHub emails a scheduled workflow's failures to
7+
# whoever last edited its cron line, so no `issues: write` is needed. The
8+
# thresholds are in docs/source/contributing/workflows.rst, the response to one
9+
# in conventions/process.rst.
1210
#
13-
# Deliberately not a `workflow_run` on Unit Tests: that would add a job per run
14-
# to a runner pool already measured saturated at 20 concurrent jobs.
11+
# Not a `workflow_run` on Unit Tests: that adds a job per run to a runner pool
12+
# already measured saturated.
1513
on:
1614
schedule:
1715
# Mondays 07:23 UTC: off the hour, where GitHub delays scheduled runs most,
18-
# and clear of Saturday's crons, 02:00 to 18:00 since #1967, and Project
19-
# Status at 00:37.
16+
# and clear of the Saturday crons (#1967) and Project Status at 00:37.
2017
- cron: '23 7 * * 1'
2118
workflow_dispatch:
2219
inputs:
@@ -29,9 +26,8 @@ on:
2926
type: number
3027
default: 0
3128

32-
# Read-only: the runs, jobs and artifacts need `actions: read`, the workflow
33-
# files at each run's sha need `contents: read`. GITHUB_TOKEN allows 1,000 API
34-
# requests an hour per repository; a default run made 677 (#1930).
29+
# GITHUB_TOKEN allows 1,000 API requests an hour per repository; a default run
30+
# made 677 (#1930).
3531
permissions:
3632
actions: read
3733
contents: read
@@ -44,20 +40,18 @@ jobs:
4440
report:
4541
name: CI timing report
4642
runs-on: ubuntu-latest
47-
# Three nested limits, each under the next, so that a slow week ends in a
48-
# smaller report rather than none (#1930). The script stops reading jobs at
49-
# --budget-minutes and writes what it has. The step's cap is the backstop, and
50-
# sits far enough above to let a request already in flight finish its retries.
51-
# This one leaves room for the setup steps and the uploads after it. Measured
52-
# for #1930, a full sample took 16.6 minutes, nearly all of it job reads, which
53-
# the API answers at up to 2.4 s each and which are made one at a time.
43+
# Three nested limits, so a slow week ends in a smaller report rather than
44+
# none (#1930): the script stops reading jobs at --budget-minutes and writes
45+
# what it has; the step's cap sits far enough above it for an in-flight
46+
# request to finish its retries; this one leaves room for setup and uploads.
47+
# A full sample took 16.6 minutes for #1930, nearly all of it serial job
48+
# reads at up to 2.4 s each.
5449
timeout-minutes: 40
5550

5651
steps:
5752
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
5853

59-
# #1660: `latest` from .github/python-versions.toml, read with the
60-
# runner's own python3.
54+
# #1660: `latest` from .github/python-versions.toml.
6155
- name: Read the Python versions
6256
id: python-versions
6357
run: python3 util/python_versions.py --github-output
@@ -66,8 +60,7 @@ jobs:
6660
with:
6761
python-version: ${{ steps.python-versions.outputs.latest }}
6862

69-
# The script fetches with requests and parses the JUnit artifacts with lxml
70-
# (#1894), and needs nothing else outside the standard library.
63+
# lxml parses the JUnit artifacts (#1894).
7164
- name: Install requests and lxml
7265
run: python -m pip install requests lxml
7366

@@ -106,8 +99,7 @@ jobs:
10699
--budget-minutes 30 --out report.json --summary report.md
107100
108101
# `always()`, not `!cancelled()`, here and below: a report written before a
109-
# cap stopped the job is still worth having (#1930), and with none written
110-
# there is nothing to add.
102+
# cap stopped the job is still worth having (#1930).
111103
- name: Add the report to the job summary
112104
if: ${{ always() }}
113105
run: |

‎.github/workflows/codeql-analysis.yml‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,23 @@ on:
44
push:
55
branches: [main]
66
pull_request:
7-
# The branches below must be a subset of the branches above
87
branches: [main]
98
schedule:
109
- cron: '0 2 * * 6'
1110

1211
# #1968: no scopes by default. `analyze` declares the ones it needs.
1312
permissions: {}
1413

15-
# Only pull-request runs may be superseded -- see the note in unit-tests.yml. A
16-
# ref-keyed group drops a *pending* run when a newer one joins it, which for a
17-
# security scan means a commit reaching main with no analysis on record.
14+
# Only pull-request runs may be superseded (see unit-tests.yml's
15+
# `concurrency`): a ref-keyed group drops a *pending* run when a newer one
16+
# joins, which here would leave a commit on main with no analysis on record.
1817
concurrency:
1918
group: codeql-${{ github.event_name == 'pull_request' && github.ref || github.run_id }}
2019
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
2120

2221
jobs:
23-
# This workflow used to call ./.github/workflows/unit-tests.yml and gate
24-
# `analyze` behind it. That re-ran the entire unit and integration matrix
25-
# against a commit the Unit Tests workflow is already testing, and it bought
26-
# nothing: CodeQL reads the source, so its findings do not depend on the test
27-
# suite passing. Worse, it meant a failing test suppressed the security
28-
# analysis for that commit. Static analysis runs on its own.
22+
# Not gated on Unit Tests: CodeQL reads the source, so its findings do not
23+
# depend on the tests passing, and a failing test would suppress the scan.
2924
analyze:
3025
name: Analyze
3126
runs-on: ubuntu-latest
@@ -38,28 +33,11 @@ jobs:
3833
- name: Checkout repository
3934
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
4035

41-
# Initializes the CodeQL tools for scanning.
4236
- name: Initialize CodeQL
4337
uses: github/codeql-action/init@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0
44-
# Override language selection by uncommenting this and choosing your languages
45-
# with:
46-
# languages: go, javascript, csharp, python, cpp, java
4738

48-
# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
49-
# If this step fails, then you should remove it and run the build manually (see below)
5039
- name: Autobuild
5140
uses: github/codeql-action/autobuild@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0
5241

53-
# ℹ️ Command-line programs to run using the OS shell.
54-
# 📚 https://git.io/JvXDl
55-
56-
# ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
57-
# and modify them (or add more) to build your code if your project
58-
# uses a compiled language
59-
60-
#- run: |
61-
# make bootstrap
62-
# make release
63-
6442
- name: Perform CodeQL Analysis
6543
uses: github/codeql-action/analyze@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0

‎.github/workflows/coverage-comment.yml‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
name: Coverage Comment
22

3-
# Posts the coverage the `test` job's `latest` leg measured in
4-
# unit-tests.yml as a pull-request comment. It runs no tests. The comment cannot
5-
# be posted from unit-tests.yml itself: that file is also a reusable workflow,
6-
# and a called workflow can only narrow the token its caller passes, never widen
7-
# it. Every caller grants `contents: read`, so asking for
8-
# `pull-requests: write` there would break every one of them, the release gate
9-
# included.
3+
# Posts the coverage the `test` job's `latest` leg measured in unit-tests.yml
4+
# as a pull-request comment. Not from unit-tests.yml itself: that file is also a
5+
# reusable workflow whose callers grant `contents: read`, and a called workflow
6+
# can only narrow its caller's token, so `pull-requests: write` there would
7+
# break every caller, the release gate included.
108
#
119
# `workflow_run` runs this file as it is on the default branch, with a token of
1210
# its own, so fork pull requests get a comment too. The price is that the

0 commit comments

Comments
 (0)