Skip to content

Make misspell lint actionable: PR-only, blocking, no false positives - #33779

Open
adrastogi wants to merge 1 commit into
mainfrom
adityar-microsoft-fix-lint-misspell-on-push
Open

adrastogi wants to merge 1 commit into
mainfrom
adityar-microsoft-fix-lint-misspell-on-push

Conversation

@adrastogi

Copy link
Copy Markdown
Contributor

Description

Reconfigures the misspell step of the Optional Lint job in .github/workflows/lint.yml:

  • Run the optional-lint job only on pull_request (both of its steps use reviewdog's github-pr-check reporter).
  • Drop locale: "US" so valid UK spellings (behaviour, cancelled, the public CANCELLED status code, ...) are accepted.
  • Ignore false positives (case-insensitive): nto (NTO = NodesToOptimize alias in optimizer code), spectre (the CPU vulnerability, flagged as a UK spelling of "specter"), interm (s390x MLAS variable).
  • Exclude .git, package-lock.json, and the vendored/minified DocFX templates under csharp/ApiDocs/_exported_templates.
  • Switch to filter_mode: added + fail_level: any so new typos fail the PR check instead of accumulating on main.

The remaining 37 genuine typos on main are fixed in a companion PR.

Validation (local, using the checksum-verified golangci/misspell v0.8.0 Windows binary, the version pinned in the action's Docker image, over git ls-files):

  • Current config (-locale US): 280 findings, matching CI's 278 (reported too many annotation (N=278)).
  • New config (-i nto,spectre,interm, same excludes): 37 findings, all genuine typos (fixed in the companion PR).
  • Exclude patterns checked with find . -not -path ... (Git Bash): no .git/, package-lock.json, or docfx.vendor.js paths remain.
  • python -c "import yaml; yaml.safe_load(open('.github/workflows/lint.yml'))": passed.

Motivation and Context

Every push to main produces a Lint failure email (300+ consecutive failed runs, e.g. https://github.com/microsoft/onnxruntime/actions/runs/41116362817). On push there is no PR diff, so filter_mode: diff_context filters nothing and misspell reports every existing finding repo-wide. reviewdog then falls back to log annotations, and its GitHubActionLogWriter.Flush returns an error whenever more than 9 results are reported (reviewdog: Too many results (annotations) in diff / reported too many annotation (N=278)), regardless of fail_level. The Optional Lint C++ job hits the same error but is masked by continue-on-error: true.

The Optional Lint job has failed on every push to main for weeks. On
push there is no PR diff, so reviewdog reports every pre-existing
finding (~278); its log-annotation fallback errors out when more than 9
are reported, regardless of fail_level.

- Run the job only on pull_request (both steps use the PR-check reporter).
- Drop locale US so valid UK spellings (behaviour, cancelled, the
  CANCELLED status code, ...) aren't flagged.
- Ignore false positives: NTO (NodesToOptimize alias), Spectre (the
  CPU vulnerability), interm (s390x MLAS variable).
- Exclude .git, package-lock.json and vendored DocFX templates.
- Use filter_mode added + fail_level any so new typos fail the PR
  instead of silently accumulating on main.

The remaining 37 genuine typos are fixed in a separate PR.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 31, 2026 01:35

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.

🟢 Approval recommended

No active review issues were identified for the targeted pull-request spell-check changes.

No open issues 🎉

What changed in this PR

Updates Optional Lint so misspell checks focus on newly added PR content and fail on new typos without reporting known false positives.

Changes:

  • Runs Optional Lint only for pull requests.
  • Accepts US and UK spellings, ignores three known false positives, and excludes Git metadata, lockfiles, and vendored DocFX templates.
  • Uses added-line filtering and fails the misspell step on any reported finding.
File Description
.github/​workflows/​lint.yml Restricts Optional Lint to PRs and configures targeted, blocking spell checks.

Review effort: Balanced

This branch has not been deployed

No deployments
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