Skip to content

Comments

Spell check only specification/**, match spell checking behavior with SpellCheck - All#32195

Merged
danieljurek merged 20 commits intomainfrom
djurek/spellcheck-exlcusions
Jan 17, 2025
Merged

Spell check only specification/**, match spell checking behavior with SpellCheck - All#32195
danieljurek merged 20 commits intomainfrom
djurek/spellcheck-exlcusions

Conversation

@danieljurek
Copy link
Member

@danieljurek danieljurek commented Jan 16, 2025

Current cspell config is generating too many errors in non specification/ folders. Most value is created when cspell scans specification/. This PR scopes cspell to check only files in the specification/ folder.

cspell does not support !specification/** so the exclusions are crude. If a PR is opened which introduces a new file into the root of the repo, spell check will fail on spelling error detections.

Tests

SpellCheck

Errors in both specification/ (included) and documentation/ (excluded) folder (note only errors described are in specification/ and none from documentation/: https://github.com/Azure/azure-rest-api-specs/actions/runs/12822906068/job/35756574727?pr=32195#step:3:41

image

No errors in PR: https://github.com/Azure/azure-rest-api-specs/actions/runs/12822920970/job/35756614201

SpellCheck - All

Spell Check - All with errors in both specification/ and documentation/ (note only errors described are in specification/ and not documentation/): https://github.com/Azure/azure-rest-api-specs/actions/runs/12822909744/job/35756584823#step:3:34414

image

Spell Check - All with no errors: https://github.com/Azure/azure-rest-api-specs/actions/runs/12822928529/job/35756633557

Other info

Another perk: because the cspell.yaml and cspell.json files are still in the root of the repo you get spell checking of files outside of the specification/ cone in vscode if you have the spell check extension:

image

Globbing does not fully support negations because the node-glob library used does not: https://cspell.org/docs/globs/ (search for !)

The ignorePaths in cspell.yaml involving directories are evaluated from the root of the config file... So node_modules/** will not exclude node_modules/** folders found in specification/, it will only exclude them if they're found in the same folder as the declaring cspell.yaml that excludes them. However, filenames like cspell.yaml are ignored wherever they are found.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 16, 2025

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jan 16, 2025

PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment.

@danieljurek danieljurek changed the title Djurek/spellcheck-exlcusions Spell check only specification/** Jan 16, 2025
@danieljurek danieljurek self-assigned this Jan 16, 2025
@danieljurek danieljurek changed the title Spell check only specification/** Spell check only specification/**, match spell checking behavior with SpellCheck - All Jan 16, 2025
@danieljurek danieljurek marked this pull request as ready for review January 16, 2025 23:46
cspell.yaml Outdated
- cspell.yaml
- node_modules/**

# Exclude all files in the repo which aren't in specification/. cspell does
Copy link
Member

@mikeharder mikeharder Jan 16, 2025

Choose a reason for hiding this comment

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

Can we use glob pattern * to match all files in root without listing them individually? I agree we'll need to list each folder in root using foo/**.

…umentation to test error behavior"

This reverts commit 5ee5ea7.
@danieljurek danieljurek enabled auto-merge (squash) January 17, 2025 05:20
@danieljurek danieljurek merged commit be2fd48 into main Jan 17, 2025
30 checks passed
@danieljurek danieljurek deleted the djurek/spellcheck-exlcusions branch January 17, 2025 05:23
ZhidaLiu pushed a commit to ZhidaLiu/azure-rest-api-specs that referenced this pull request Jan 22, 2025
… SpellCheck - All (Azure#32195)

Spell check only specification/**, match spell check behavior with SpellCheck - All
markcowl pushed a commit to markcowl/azure-rest-api-specs that referenced this pull request Feb 6, 2025
… SpellCheck - All (Azure#32195)

Spell check only specification/**, match spell check behavior with SpellCheck - All
najian pushed a commit to najian/azure-rest-api-specs that referenced this pull request Mar 4, 2025
… SpellCheck - All (Azure#32195)

Spell check only specification/**, match spell check behavior with SpellCheck - All
pjpatel12 pushed a commit to pjpatel12/azure-rest-api-specs that referenced this pull request Apr 29, 2025
… SpellCheck - All (Azure#32195)

Spell check only specification/**, match spell check behavior with SpellCheck - All
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants