Skip to content

docs: linting (vale, markdownlint-cli2, lychee) of our documentation as part of the CI

Pawel Rozlach requested to merge prozlach/markdown-lint into master

What does this MR do?

Related to #1070 (closed)

This MR adds linting stage to CI: vale (prose), markdownlint-cli2 (Markdown formatting), lychee (links) and brings us on-par with standards and conventions that are used in gitlab repo or e.g. gitlab-runner repo.

I have copied markdown rules rules from https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.markdownlint?ref_type=heads, revision 10235fc0c61d. Vale rules come from https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.vale?ref_type=heads, same commit with the exception of 3 rules that did not seem to make sense for us:

  • docs/.vale/gitlab_docs/HistoryItems.yml
  • docs/.vale/gitlab_docs/InternalLinksCode.yml
  • docs/.vale/gitlab_docs/RelativeLinks.yml

plus rules that do not work on the standard tooling container we use:

  • docs/.markdownlint/rules/tabs_blank_lines.js
  • docs/.markdownlint/rules/tabs_title_markup.js
  • docs/.markdownlint/rules/tabs_title_text.js

Instead of reviewing it, you can just verify if the rules match the ones in gitlab repo

Script automation was heavily inspired by how gitlab-runner does it.

I have opted for one big MR that fixes all the validation errors and makes the make lint-docs target pass. This way there is only one standard we will be adhering to. I eyeballed the changes and did not notice any markdown rendering errors, but there were so many fixes that I could have missed something so I would appreciate second pair of eyes.

Thanks in advance for the patience reviewing this MR :)

Author checklist

  • Feature flags
    • Added feature flag:
    • This feature does not require a feature flag
  • I added unit tests or they are not required
  • I added documentation (or it's not required)
  • I followed code review guidelines
  • I followed Go Style guidelines
  • For database changes including schema migrations:
    • Manually run up and down migrations in a postgres.ai production database clone and post a screenshot of the result here.
    • If adding new queries, extract a query plan from postgres.ai and post the link here. If changing existing queries, also extract a query plan for the current version for comparison.
      • I do not have access to postgres.ai and have made a comment on this MR asking for these to be run on my behalf.
    • Do not include code that depends on the schema migrations in the same commit. Split the MR into two or more.
  • Ensured this change is safe to deploy to individual stages in the same environment (cny -> prod). State-related changes can be troublesome due to having parts of the fleet processing (possibly related) requests in different ways.

Reviewer checklist

  • Ensure the commit and MR tittle are still accurate.
  • If the change contains a breaking change, apply the breaking change label.
  • If the change is considered high risk, apply the label high-risk-change
  • Identify if the change can be rolled back safely. (note: all other reasons for not being able to rollback will be sufficiently captured by major version changes).

If the MR introduces database schema migrations:

  • Ensure the commit and MR tittle start with fix:, feat:, or perf: so that the change appears on the Changelog
If the changes cannot be rolled back follow these steps:
  • If not, apply the label cannot-rollback.
  • Add a section to the MR description that includes the following details:
    • The reasoning behind why a release containing the presented MR can not be rolled back (e.g. schema migrations or changes to the FS structure)
    • Detailed steps to revert/disable a feature introduced by the same change where a migration cannot be rolled back. (note: ideally MRs containing schema migrations should not contain feature changes.)
    • Ensure this MR does not add code that depends on these changes that cannot be rolled back.
Edited by Pawel Rozlach

Merge request reports

Loading