Skip to content

ci: Change the spell detection strategy#13472

Open
sylvestre wants to merge 2 commits into
uutils:mainfrom
sylvestre:spell
Open

ci: Change the spell detection strategy#13472
sylvestre wants to merge 2 commits into
uutils:mainfrom
sylvestre:spell

Conversation

@sylvestre

Copy link
Copy Markdown
Contributor

I think we wasted too much time on typo detection. Let's avoid failing the build just for that

The local hook was language: system and exited 0 with a "cspell not found,
skipping spell check" message when cspell was absent from PATH, which
pre-commit reported as "Passed". In practice nobody had cspell installed, so
spelling was only ever checked in CI.

Use the upstream cspell-cli hook instead: it is language: node, so pre-commit
installs cspell itself and cannot silently skip. Pin language_version since
cspell v10 requires node >= 22.18.
Now that cspell runs locally via pre-commit, Style/spelling only annotates
unknown words rather than blocking the merge.

Also fix the pre_commit job, which ran "pre-commit run" without --all-files
and so matched zero files on a fresh checkout. Skip the hooks already covered
by the Style/format and Style/lint jobs, and drop the npm install of cspell
now that the hook provides its own.
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/tail/truncate. tests/tail/truncate is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!

# Spelling is enforced locally by the cspell pre-commit hook, where it is
# cheap to fix. Here it is advisory: unknown words show up as annotations
# on the diff instead of blocking the merge.
STYLE_FAIL_ON_FAULT: false

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.

Hm, is this job still necessary if it never fails?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants