chore(cdk-lint): add linting to README.md files #11708

RomainMuller · 2020-11-25T15:22:47Z

Consistent heading structure in the README.md files is necessary in
order to ensure consistent rendering in the Python documentation, as the
rendering of the TOC tree is dependent on heading levels. The best
experience is achieved by having all README.md files start with a H1
heading, and subsequently contain only H2 and deeper headings. It is
also important that no heading level is skipped (i.e: a H4 "nested" in
a H2).

This adds a cdk-lint stage that executes markdownlint on the
README.md files, with a configuration that is crafted specifically to
maximize compatibility with most Markdown parsers, and to ensure
correct rendering in the Python docs. Many of these offenses can be
automatically fixed by running cdk-lint --fix or yarn lint --fix, however
some cannot be automatically fixed by the tool and will require human
intervention.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Consistent heading structure in the `README.md` files is necessary in order to ensure consistent rendering in the Python documentation, as the rendering of the TOC tree is dependent on heading levels. The best experience is achieved by having all `README.md` files start with a `H1` heading, and subsequently contain only `H2` and deeper headings. It is also important that no heading level is skipped (i.e: a `H4` "nested" in a `H2`). This adds a `pkglint` rule that parses the `README.md` files using the `commonmark` parser, and ensures the heading structure is consistent per the above rules, and registers fixable offenses otherwise. Each offense is reported (and fixed) independently in an effort to provide "more actionnable" feedback in the context of PR validation, and to avoid risking to modify other parts of the document if we were to simply "re-render" the updated Markdown.

gitpod-io · 2020-11-25T15:22:50Z

RomainMuller · 2020-11-25T15:25:43Z

Note: this probably should be merged only after https://github.com/aws/cdk-ops/pull/945 has been merged.

tools/pkglint/lib/rules.ts

RomainMuller · 2020-11-26T13:39:20Z

packages/@aws-cdk/aws-apigatewayv2/README.md

+Features                                   | Stability
+-------------------------------------------|--------------------------------------------------------
+CFN Resources                              | ![Stable](https://img.shields.io/badge/stable-success.svg?style=for-the-badge)
+Higher level constructs for HTTP APIs      | ![Experimental](https://img.shields.io/badge/experimental-important.svg?style=for-the-badge)
+Higher level constructs for Websocket APIs | ![Not Implemented](https://img.shields.io/badge/not--implemented-black.svg?style=for-the-badge)
+


This change is such that the table reads better when looking at it in the source. I find it's a neat improvement.

RomainMuller · 2020-11-26T13:42:07Z

tools/pkglint/lib/rules.ts

@@ -1580,5 +1594,5 @@ function toRegExp(str: string): RegExp {
 }

 function readBannerFile(file: string): string {
-  return fs.readFileSync(path.join(__dirname, 'banners', file), { encoding: 'utf-8' });
+  return fs.readFileSync(path.join(__dirname, 'banners', file), { encoding: 'utf-8' }).trim();


This trim() ensures we don't insert too much vertical space in the stability banners, as markdownlint would subsequently consider those are formatting errors.

Consider adding this comment directly into the code.

…eadme-headings

nija-at

Minor comments below.

nija-at · 2020-12-02T10:27:23Z

tools/cdk-build-tools/config/markdownlint.json

@@ -0,0 +1,30 @@
+{
+  "default": false,
+  "MD001": true,


can we use the rule name instead of the code?
Tells us which rule this is without having to reference somewhere, at least a first approximation.

We can yeah. I had initially decided against because rules have multiple names... But I guess you're right this might be better In the long run. If only JSON supported comments... -_-

nija-at · 2020-12-02T10:30:59Z

tools/cdk-build-tools/tsconfig.json

@@ -2,19 +2,19 @@
  "compilerOptions": {
    "target": "ES2018",
    "module": "commonjs",
-    "lib": ["es2018"],
+    "lib": ["dom", "es2018"],


add comment on why dom is needed.

Oh - good you'd ask... It was needed in one stage of my experiments there but is no longer needed!

nija-at · 2020-12-02T10:31:25Z

tools/cdk-build-tools/tsconfig.json

    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "resolveJsonModule": true,
    "composite": true,
-    "incremental": true
+    "incremental": true,
+    "importHelpers": true,


add a comment on why this option is needed.

nija-at · 2020-12-02T10:33:53Z

packages/@aws-cdk/aws-apigatewayv2/README.md

+Features                                   | Stability
+-------------------------------------------|--------------------------------------------------------
+CFN Resources                              | ![Stable](https://img.shields.io/badge/stable-success.svg?style=for-the-badge)
+Higher level constructs for HTTP APIs      | ![Experimental](https://img.shields.io/badge/experimental-important.svg?style=for-the-badge)
+Higher level constructs for Websocket APIs | ![Not Implemented](https://img.shields.io/badge/not--implemented-black.svg?style=for-the-badge)
+


…eadme-headings

mergify · 2020-12-02T12:00:47Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-12-02T12:30:36Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ad590f5
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-12-02T12:30:53Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

RomainMuller added the docs/generated Related to the generated API Reference documentation label Nov 25, 2020

RomainMuller requested a review from a team November 25, 2020 15:22

RomainMuller self-assigned this Nov 25, 2020

mergify bot added the contribution/core This is a PR that came from AWS. label Nov 25, 2020

RomainMuller added the pr/do-not-merge This PR should not be merged at this time. label Nov 25, 2020

nija-at suggested changes Nov 25, 2020

View reviewed changes

tools/pkglint/lib/rules.ts Outdated Show resolved Hide resolved

use markdownlint instead of rolling my own

727e1d6

RomainMuller requested a review from nija-at November 26, 2020 13:38

RomainMuller commented Nov 26, 2020

View reviewed changes

fix unit test

81b5468

RomainMuller changed the title ~~chore(pkglint): rule to ensure heading consistency in README.md~~ chore(cdk-lint): add listing to README.md files Nov 26, 2020

lint ubergen properly

05ffcd0

RomainMuller changed the title ~~chore(cdk-lint): add listing to README.md files~~ chore(cdk-lint): add linting to README.md files Nov 30, 2020

RomainMuller added 6 commits November 30, 2020 10:23

correctly lint rewrite-imports

af1b730

Merge remote-tracking branch 'origin/master' into rmuller/normalize-r…

b486e5d

…eadme-headings

fix aws-signer readme

d488ebd

fix networkfirewall

f7c7aaf

Merge remote-tracking branch 'origin/master' into rmuller/normalize-r…

875d63a

…eadme-headings

fix aws-lambda-nodejs

12d850c

nija-at approved these changes Dec 2, 2020

View reviewed changes

RomainMuller added 2 commits December 2, 2020 11:54

@nija-at feedback

d273a73

Merge remote-tracking branch 'origin/master' into rmuller/normalize-r…

0da3ab5

…eadme-headings

RomainMuller removed the pr/do-not-merge This PR should not be merged at this time. label Dec 2, 2020

fixum aws-cdk/README.md

433533f

Merge branch 'master' into rmuller/normalize-readme-headings

ad590f5

mergify bot merged commit 2b1160f into master Dec 2, 2020

mergify bot deleted the rmuller/normalize-readme-headings branch December 2, 2020 12:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cdk-lint): add linting to README.md files #11708

chore(cdk-lint): add linting to README.md files #11708

RomainMuller commented Nov 25, 2020 •

edited

Loading

gitpod-io bot commented Nov 25, 2020 •

edited

Loading

RomainMuller commented Nov 25, 2020

RomainMuller Nov 26, 2020

nija-at Dec 2, 2020

RomainMuller Nov 26, 2020

nija-at Dec 2, 2020

nija-at left a comment

nija-at Dec 2, 2020

RomainMuller Dec 2, 2020

nija-at Dec 2, 2020

RomainMuller Dec 2, 2020

nija-at Dec 2, 2020

nija-at Dec 2, 2020

mergify bot commented Dec 2, 2020

aws-cdk-automation commented Dec 2, 2020

mergify bot commented Dec 2, 2020

chore(cdk-lint): add linting to README.md files #11708

chore(cdk-lint): add linting to README.md files #11708

Conversation

RomainMuller commented Nov 25, 2020 • edited Loading

gitpod-io bot commented Nov 25, 2020 • edited Loading

RomainMuller commented Nov 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nija-at left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Dec 2, 2020

aws-cdk-automation commented Dec 2, 2020

AWS CodeBuild CI Report

mergify bot commented Dec 2, 2020

RomainMuller commented Nov 25, 2020 •

edited

Loading

gitpod-io bot commented Nov 25, 2020 •

edited

Loading