Unify documentation style #585
Conversation
There was a problem hiding this comment.
+@MarcusSorealheis +@blakehatch
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable (waiting on @blakehatch and @MarcusSorealheis)
|
Such a massive change. I'm sure the code change is good. To make the PR better, it would be good if the text here was explaining a bit more what is going on and how things change. I'm sure some of the PR template was not relevant, but some was. It's important for us to understand what's going on. |
| @@ -7,7 +7,7 @@ Fixes # (issue) | |||
|
|
|||
| ## Type of change | |||
|
|
|||
| Please delete options that are not relevant. | |||
| Please delete options that aren't relevant. | |||
There was a problem hiding this comment.
for example, is this automated, is it related to the change? Is it its own change. Does the style change remove all contractions? What is this Microsoft folder? Is there a link to what they're acomplishing?
There was a problem hiding this comment.
I've updated the commit message to include more information.
A common way to use Vale is to run a vale sync when initially cloning the repository to get the configured style lings like the Microsoft lints. We're deliberately not doing that here and instead vendor the styles since that obsoletes any manual Vale setup and makes the hooks reproducible.
The lints run as part of the pre-commit hooks and will currently only fail on things like typos. Additionally, the error messages are added as inline comments in PRs (looks like this: https://user-images.githubusercontent.com/8785025/85236358-272d3680-b3d2-11ea-8793-0f45cb70189a.png).
It's possible to get a very large amount of additional suggestions by running vale manually on files as described in the updated CONTRIBUTING.md. Those warnings and suggestions were so many that I had to postpone fixes for those.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable (waiting on @blakehatch and @MarcusSorealheis)
.github/pull_request_template.md line 10 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
for example, is this automated, is it related to the change? Is it its own change. Does the style change remove all contractions? What is this Microsoft folder? Is there a link to what they're acomplishing?
Vale has three warning levels: error, warning and suggestion. These changes are manual fixes for the "error" level that Vale raises in CI.
The Microsoft warnings come from here: https://github.com/errata-ai/Microsoft
The reason contractions specifically are flagged is outlined here: https://learn.microsoft.com/en-us/style-guide/word-choice/use-contractions
There was a problem hiding this comment.
If this only fails on typos this honestly only seems like an upside to me and we can ramp up use of the optional rules as we see fit.
Reviewed 82 of 85 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @blakehatch and @MarcusSorealheis)
There was a problem hiding this comment.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @blakehatch)
Obsolete many human-written review comments related to typos and text style issues. Instead, a bot autocomments on such issues now. The implicit style guide introduced in this commit is based on Microsoft's style guide (https://learn.microsoft.com/en-us/style-guide/welcome/), with a few enhancements that aim to improve readability and conciseness. This commit fixes a large number of typos. Warnings and suggestions are not handled in CI yet and need to be addressed in subsequent commits. We might need to make adjustments to the finer-grained warnings, so except for hard errors everythinng else is disabled at the moment.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Dismissed @MarcusSorealheis from a discussion.
Reviewable status:complete! 1 of 1 LGTMs obtained
Largely obsolete review comments on typos and text style issues by offloading them to Vale and a CI bot.
This commit fixes a large number of typos. Warnings and suggestions are not handled in CI yet and need to be addressed in subsequent commits.
This change is