Skip to content

Commit

Permalink
A few more CI workflow comment and style improvements
Browse files Browse the repository at this point in the history
These didn't make it into #1668.

Besides comments, the changes are for consistency with the
prevailing style, usually by omitting redundant YAML quoting.

Removal of the outer double quotes in the `if` in `tests-pass` is a
case of this, and produces an equivalent node in parsing (i.e. its
equivalence does not depend on anything about GHA itself). But just
to be sure, I did run

    yq '.jobs.tests-pass.steps[0].if' .github/workflows/ci.yml

before and after the change, to ensure the output was the same.

The other change here that deserves comment is the removal of `--`
as an argument to a `diff` command. When any path argument is
formed from paramter expansion or from a glob with a leading `*` or
other globbing character, `--` helps express that the following
arguments are not options. For `git diff`, a `--` expresses that
the following arguments are neither options nor refs, but paths, so
all `git diff` commands with paths in the CI workflows use `--`
even if no shell expansions are involved.

(In practice this means `--` is often useful for `diff` with paths
and, based on this habit, I had inadvertently written a `--` where
neither of the above scenarios applied. But that had actually
decreased stylistic consistency because we are not using `--`
elsewhere that the meaning of all arguments after it is unambiguous
even without examining any surrounding context.)
  • Loading branch information
EliahKagan committed Nov 12, 2024
1 parent 0155aec commit cb16d94
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 9 deletions.
15 changes: 7 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,14 @@ jobs:
- uses: Swatinem/rust-cache@v2
- name: Setup dependencies (macos)
if: startsWith(matrix.os, 'macos')
run:
brew install tree openssl gnu-sed
run: brew install tree openssl gnu-sed
- name: "cargo check default features"
if: startsWith(matrix.os, 'windows')
run: cargo check --workspace --bins --examples
- uses: taiki-e/install-action@v2
with:
tool: nextest
- name: "Test (nextest)"
- name: Test (nextest)
env:
GIX_TEST_CREATE_ARCHIVES_EVEN_ON_CI: '1'
run: cargo nextest run --workspace --no-fail-fast
Expand Down Expand Up @@ -157,9 +156,9 @@ jobs:
- uses: taiki-e/install-action@v2
with:
tool: cross
- name: "check"
- name: check
run: cross check -p gix --target ${{ matrix.target }}
- name: "Test (unit)"
- name: Test (unit)
# run high-level unit tests that exercise a lot of code while being pure Rust to ease building test binaries.
# TODO: figure out why `git` doesn't pick up environment configuration so build scripts fail when using `-p gix`.
run: cross test -p gix-hashtable --target ${{ matrix.target }}
Expand Down Expand Up @@ -275,7 +274,7 @@ jobs:

defaults:
run:
shell: bash
shell: bash # Use bash even on Windows, if we ever reenable windows-latest for testing.

steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -327,7 +326,7 @@ jobs:
- name: Each job must block PRs or be declared not to
run: |
sort -m blocking-jobs.txt expected-nonblocking-jobs.txt |
diff --color=always -U1000 -- - all-jobs.txt
diff --color=always -U1000 - all-jobs.txt
# Dummy job to have a stable name for the "all tests pass" requirement
tests-pass:
Expand All @@ -349,7 +348,7 @@ jobs:

steps:
- name: Fail if ANY dependency has failed or cancelled
if: "contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')"
if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
run: exit 1
- name: OK
run: exit 0
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:
workflow_dispatch:

permissions:
contents: read # Set more permissively in jobs that need `write`.
contents: read # This is set more permissively in jobs that need `write`.

defaults:
run:
Expand Down

0 comments on commit cb16d94

Please sign in to comment.