Skip to content

fix(pre-commit): install vale-sync from the cmd/vale package - #14753

Merged
DonLakeFlyer merged 2 commits into
mavlink:masterfrom
TSC21:fix/pre-commit-vale-sync-install
Aug 9, 2026
Merged

fix(pre-commit): install vale-sync from the cmd/vale package#14753
DonLakeFlyer merged 2 commits into
mavlink:masterfrom
TSC21:fix/pre-commit-vale-sync-install

Conversation

@TSC21

@TSC21 TSC21 commented Jul 31, 2026

Copy link
Copy Markdown
Member

RIIS

Contributed on behalf of RIIS, LLC.



CI/Build Changes

Two commits to the pre-commit configuration:

  1. The vale-sync hook installs its Go dependency from github.com/errata-ai/vale/v3/cmd/vale instead of the module root. The module root does not contain a Go package (the vale binary lives under cmd/vale), so go install fails and pre-commit aborts the entire run before a single hook fires.
  2. Three inline bash hook entries (check-categorized-logging, check-no-qtest-ignore-message, check-no-fixed-qwait) are folded into YAML block scalars because they run past the 200 column limit that yamllint enforces, so the config was failing its own gate as soon as the file gets linted. YAML folds the lines with single spaces, so the entries parse to exactly the same command as before.

Reason

Right now the whole pre-commit gate is silently dead: the broken vale-sync environment install makes pre-commit exit with code 3 and zero hook verdicts, everywhere this config runs, CI included. The error is:

go: module github.com/errata-ai/vale/v3@v3.14.2 found, but does not contain package github.com/errata-ai/vale/v3

So clang-format, typos, qmllint, actionlint and everything else in the config have not actually been running. I found this while chasing a pre-commit results artifact that reported exit_code: 3, passed: 0, failed: 0 on a fork. There is a companion PR that makes the workflow fail loudly when this class of breakage happens again.

Testing

  • Tested workflow locally (ran the pre-commit runner itself with this config: it installs the environment and produces hook verdicts again)
  • Verified YAML syntax (check-yaml and yamllint pass on the file)
  • Tested on fork before submitting (the same fix is running on my fork, and each folded hook was checked to still exit 1 on a synthetic violation and 0 on a clean file)

Impact

The pre-commit workflow on every PR, and every local pre-commit run. No workflow files change in this PR.

Checklist

  • I have read the Contribution Guidelines
  • Workflow permissions follow least-privilege principle (no workflow changes here)
  • No secrets are exposed in logs

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

TSC21 added 2 commits July 31, 2026 10:51
The vale module root carries no Go package (the vale binary lives
under cmd/vale), so the hook's environment install fails:

    go: module github.com/errata-ai/vale/v3@v3.14.2 found, but does
    not contain package github.com/errata-ai/vale/v3

pre-commit aborts the whole run on the install error before a single
hook fires and exits with code 3, so every hook in the config is
silently skipped wherever this configuration runs, the CI runner
included. Installing the cmd/vale package restores the gate.
Three inline-bash hook entries run past the 200 column line limit
yamllint enforces, so the configuration fails its own gate the first
time it is linted as a changed file. Folded block scalars carry the
same commands: YAML joins the folded lines with single spaces, so
the entries shlex to byte-identical argv.
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Build results unavailable — artifact download from one or more platform workflows failed (likely artifact retention expiry or transient API error). The combined report cannot be generated for this run.

See the Build Results workflow run for details.

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

@TSC21 Yikes, this was nasty. Thanks so much for finding/fixing.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes pre-commit initialization and YAML lint compliance.

Changes:

  • Installs Vale from its executable package path.
  • Folds long Bash hook commands without changing behavior.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

One request before merge: please align the vale version pins while you're in here. vale-sync installs github.com/errata-ai/vale/v3/cmd/vale@v3.14.2, but the vale lint hook below it uses repo rev v3.15.1, so the styles are synced with a different vale version than the one that lints. It's pre-existing drift rather than something this PR introduced, but since this PR is already touching the vale-sync dependency line, bumping it to @v3.15.1 keeps the two in lockstep.

@DonLakeFlyer

Copy link
Copy Markdown
Contributor

One request before merge: please align the vale version pins while you're in here.

@TSC21 Don't worry about this, I'll create a separate pull for that.

@DonLakeFlyer
DonLakeFlyer merged commit 0e16620 into mavlink:master Aug 9, 2026
37 checks passed
DonLakeFlyer added a commit that referenced this pull request Aug 9, 2026
vale-sync installed vale@v3.14.2 while the vale lint hook uses
v3.15.1, so styles were synced with a different vale version than
the one that lints. Bump the vale-sync dependency to v3.15.1 to
keep the two in lockstep.

Follow-up to #14753.
DonLakeFlyer added a commit to DonLakeFlyer/qgroundcontrol that referenced this pull request Aug 9, 2026
vale-sync installed vale@v3.14.2 while the vale lint hook uses
v3.15.1, so styles were synced with a different vale version than
the one that lints. Bump the vale-sync dependency to v3.15.1 to
keep the two in lockstep.

Follow-up to mavlink#14753.
DonLakeFlyer added a commit that referenced this pull request Aug 9, 2026
vale-sync installed vale@v3.14.2 while the vale lint hook uses
v3.15.1, so styles were synced with a different vale version than
the one that lints. Bump the vale-sync dependency to v3.15.1 to
keep the two in lockstep.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants