Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add spell check tools to eng/common #1421

Merged
8 commits merged into from
Feb 23, 2021
Merged

Add spell check tools to eng/common #1421

8 commits merged into from
Feb 23, 2021

Conversation

danieljurek
Copy link
Member

@danieljurek danieljurek commented Feb 18, 2021

See example run in Embedded C repo -- https://dev.azure.com/azure-sdk/public/_build/results?buildId=739511&view=logs&j=a129effc-2dd1-54d1-fb5a-ad7bdc0e851d&t=7b4713e3-e110-5049-e231-cc8a1e664c68

Logs:
image

Pipeline warnings:
image

check-spelling.yml

  • Runs only on PRs
  • Fetches the "base" ref (usually the master branch) of the PR.. this takes some work as the git config in DevOps is not set to match branch names the way a typical git clone would
  • Runs Test-Spelling.ps1
  • Needs to be wired into a build job

Test-Spelling.ps1 is designed to run on a dev box as well as in DevOps. It will advise if there are errors.

I would have preferred to put cspell.json in the eng/ folder in each repo. However the configuration in the vcsode plugin searches only in the root of the repo, the .vscode folder, or specific preferences set in .vscode/settings.json (or at a user level). If we want to keep the root of the repo less cluttered then it makes sense to put cspell.json into .vscode (see demo PR).

Of note: there are notes in the vscode extension repo about possibly using a cspell.configLocation setting but that functionality does not yet exist.

@danieljurek danieljurek added the Central-EngSys This issue is owned by the Engineering System team. label Feb 18, 2021
@danieljurek danieljurek requested a review from a team as a code owner February 18, 2021 06:15
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@danieljurek danieljurek force-pushed the spellcheck branch 2 times, most recently from 0ae03b3 to 3b79a5f Compare February 19, 2021 07:32
* Use common approach to resolving base branch name
* Eliminate default base branch "master" as this will be changed later, providing no default and using a mandatory parameter means a dev must provide the value
* Check for existence of $CspellConfigPath
* No need to Set-Location if run from the root of the repo (most common scenario)
* -join
This reverts commit 88b382a.
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

…Add reference to spelling docs, exit 0, Rename to check-spelling-in-changed-files.ps1,
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

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

I couple small clean-up comments but otherwise looks good.

… validation farther down that will also catch these errors), Update comments and script name
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

@ghost
Copy link

ghost commented Feb 23, 2021

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 1bced20 into Azure:master Feb 23, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants