Skip to content

Apply pre-commit formatting and enforce it in CI #333

Description

@mcocdawc

eckit has a .pre-commit-config.yaml, but nothing in CI ever runs it — neither
.github/workflows/ci.yml nor .ci/manifest.toml invokes pre-commit. So the config is
advisory only, and the tree has drifted away from it.

Two things to do

1. Apply the hooks once. Running pre-commit run --all-files today rewrites ~19 files:
import reordering (docs/conf.py), trailing whitespace (src/eckit/sql/sqll.l,
sqly.y, SchemaAnalyzer.cc), missing/extra final newlines (tests/parser/*.yaml),
plus python/, src/sandbox/ and tests/ files. Best landed as its own
formatting-only commit so it stays reviewable and doesn't mix with behaviour changes.

2. Fix yamllint, then enforce in CI. yamllint runs with --strict, so warnings are
errors, and it currently fails on GitHub workflow files:

.github/workflows/ci.yml
  1:1  warning  missing document start "---"  (document-start)
  3:1  warning  truthy value should be one of [false, true]  (truthy)

The truthy one is inherent to workflow YAML — GitHub requires a bare on: key, which
yamllint reads as the boolean true. Quoting it as "on": would satisfy the linter but
edits the trigger block, so excluding .github/workflows/ from yamllint (or relaxing
those two rules there) is probably the better fix.

This is not hypothetical: it currently blocks any commit that touches a workflow file,
because pre-commit runs the hook on staged files. It blocked a two-line change to
ci.yml in this repo today, which had to go in with --no-verify.

Once green, add a pre-commit job to ci.yml. There is a shared action for it:

  pre-commit:
    runs-on: ubuntu-slim
    env:
      FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true'
    steps:
      - uses: actions/checkout@v6
      - uses: ecmwf/ci-infrastructure/actions/pre-commit@main

ecflow, stack-dependencies and ci-infrastructure already use it.

Also worth a look

.pre-commit-config.yaml lists pre-commit/pre-commit-hooks twice, at two different revs
(v5.0.0 and v4.5.0), which looks like accidental drift — the other repos in the rollout
are on v6.0.0.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions