prefect icon indicating copy to clipboard operation
prefect copied to clipboard

Adds vale style guide for manual linting and updates docs to remove style guide errors

Open discdiver opened this issue 1 year ago • 2 comments

Adds Vale for style guide linting. Closes #13908

Does not run Vale on PRs, but allows users to run it manually. I'm not sure we will run as part of CI/CD, or might just run for errors and not warning, or might adjust for fewer warnings.

Uses Google's style guide as a base. Configuration and modifications are found in docs/vale.ini and docs/styles/CustomStyles where the Google WordList has been updated.

You can turn off Vale for particular files or particular parts of files, or just turn off particular rules for particular files.

Here's the HTML to exclude a rule:

<!-- vale Google.SomeRule = NO -->

my docs Markdown that I don't want the rule to apply to goes here

<!-- vale Google.SomeRule = YES -->

Vale spell-checking is disabled, as we have a spell-checking tool in use already.

Install Vale with pip install vale. The package is listed in requirements.dev as part of this PR.

Run vale docs/integrations to check the files in the integrations directory. It is recommended that you not run it on all docs files, as it will take quite a while to run.

Additional style guide plugins will likely be added in the future.

This PR fixes all Error-level findings (and some Warning level findings) outside of the API docs. API docs could be evaluated in the future.

Checklist

  • [x] This pull request includes a label categorizing the change e.g. maintenance, fix, feature, enhancement, docs.
  • [ ] This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • [ ] If this pull request adds new functionality, it includes unit tests that cover the changes
  • [ ] If this pull request removes docs files, it includes redirect settings in mint.json.
  • [ ] If this pull request adds functions or classes, it includes helpful docstrings.

discdiver avatar Jun 28 '24 22:06 discdiver

While I'm all for brevity, "app" makes me think of a consumer or mobile app, instead of the more complex B2B platform that Prefect is.

Fully agreed

billpalombi avatar Jun 28 '24 23:06 billpalombi

We can override to make it require application.

discdiver avatar Jun 28 '24 23:06 discdiver

I'm still seeing a few "app" changes that need to be removed

billpalombi avatar Jul 01 '24 13:07 billpalombi

I'm still seeing a few "app" changes that need to be removed

Good catch @billpalombi . I see what happened. Updated.

Only "apps" now are the ones that were there previously on the Slack integration page. I think the deferred-tasks FastAPI app section that references web apps could be changed from "application" to "app" if we wanted to, but that it's fine with the more formal "application"

discdiver avatar Jul 01 '24 14:07 discdiver