aio + docs/api: implement content checks #22759

petebacondarwin · 2018-03-14T09:59:27Z

This PR adds a new dgeni processor checkContentRules, which will enable us to write rules about how the content should appear. We then implement two actual rules:

prevent any headings in @description blocks and only allow h3 headings in @usageNotes blocks
prevent single character parameter names

Since this produces a large number of errors the checkContentRules.failOnContentErrors is not set to true. But we can do this later once the documentation content has been cleaned up.

Further we can add or adjust these rules going forward to ensure more documentation consistency.

Closes #22137
Closes #22682

mary-poppins · 2018-03-14T10:06:45Z

You can preview 01b2a5e at https://pr22759-01b2a5e.ngbuilds.io/.

petebacondarwin · 2018-03-14T10:23:38Z

aio/tools/transforms/angular-api-package/content-rules/noMarkdownHeadings.js

+ * To creeate a rule that will only allow level 3 headings:
+ *
+ * ```
+ * const rule = createrNoMarkdownHeadingRule(1, 2, '4,');


mary-poppins · 2018-03-14T10:32:17Z

You can preview 19e89ba at https://pr22759-19e89ba.ngbuilds.io/.

gkalpak

What about non-markdown headings (e.g. <h1>Haxx</h1>)?

gkalpak · 2018-03-14T16:43:20Z

aio/tools/transforms/angular-api-package/content-rules/noMarkdownHeadings.js

+  if (!disallowedHeadings.length) {
+    disallowedHeadings.push('#{1,}');
+  }
+  const regex = new RegExp(`^ ?(${disallowedHeadings.join('|')}) +.*$`, 'mg');


What if there are 2+ leading whitespaces?

Hmm, good question. According to the CommonMark spec:

The opening # character may be indented 0-3 spaces.

I will fix this.

gkalpak · 2018-03-14T16:59:18Z

aio/tools/transforms/angular-base-package/processors/checkContentRules.js

+ *
+ * The processor will run each rule function against each matching property of each matching doc.
+ *
+ * An example rule might look like:


"A totally made-up example..." 😛

gkalpak · 2018-03-14T17:00:00Z

aio/tools/transforms/angular-base-package/processors/checkContentRules.js

+     *     [property]: Array<(doc: Document, property: string, value: any) => string|undefined>
+     *   }
+     * }
+     */


gkalpak · 2018-03-14T17:07:09Z

aio/tools/transforms/angular-base-package/processors/checkContentRules.js

+    $runBefore: ['processing-docs'],
+    $process(docs) {
+      const errors = [];
+      // Ensure that we have ruleFns for all regex rules


Not sure what this comment refers to 😕

Doh! From a previous incarnation of the algorithm, which I abandoned.

petebacondarwin · 2018-03-14T19:36:41Z

What about non-markdown headings (e.g. <h1>Haxx</h1>)?

My view is that we let the authors override anything by using HTML directly.
This is the backdoor that stops us having to change the rules every time some DevRel person insists that we allow them to do a "one-off" :-)

This processor will enable us to write rules about how the content should appear, such as: * no headings in markdown content * only one sentence per line * no single character parameter names * etc.

This content rule can check what markdown headings appear in content properties.

* No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes`

This rule can be used to ensure that properties contain a minimum number of characters.

mary-poppins · 2018-03-14T20:16:23Z

You can preview d64e868 at https://pr22759-d64e868.ngbuilds.io/.

petebacondarwin · 2018-03-14T21:10:47Z

@gkalpak PTAL

IgorMinar

I can't tell from this code if this will results in warnings or errors, but since the build is green, I assume warnings. If that's the case then this LGTM.

IgorMinar · 2018-04-12T06:24:58Z

aio/tools/transforms/angular-api-package/content-rules/minLength.js

@@ -0,0 +1,8 @@
+module.exports = function createMinLengthRule(minLength) {
+  minLength = minLength || 2;


we should consider being a bit more aggressive and require at least 3 or 4 characters. I can't think of any variables that should be less than 4 chars, but we should test and see. I suggest we start with 4 and if we find legit usecases for variable names of length 3 then we can decrease it.

How about url :-P This occurs "lots" of times.

Others that occur in the current code:

p

req

fn

key

k

v

ctx

obj

id

el

cd

dir

c

cb

max

min

res

err

cdr

sw

I would say that setting the minimum size to 3 chars would be a reasonable compromise.

This content rule can check what markdown headings appear in content properties. PR Close #22759

* No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes` PR Close #22759

This rule can be used to ensure that properties contain a minimum number of characters. PR Close #22759

PR Close #22759

petebacondarwin · 2018-04-12T07:13:29Z

Yes we can set the checkContentRules.failOnContentErrors flag to true to fail the build.

This processor will enable us to write rules about how the content should appear, such as: * no headings in markdown content * only one sentence per line * no single character parameter names * etc. PR Close angular#22759

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

…2759) * No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes` PR Close angular#22759

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

…#22759) PR Close angular#22759

This processor will enable us to write rules about how the content should appear, such as: * no headings in markdown content * only one sentence per line * no single character parameter names * etc. PR Close angular#22759

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

…2759) * No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes` PR Close angular#22759

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

…#22759) PR Close angular#22759

This processor will enable us to write rules about how the content should appear, such as: * no headings in markdown content * only one sentence per line * no single character parameter names * etc. PR Close angular#22759

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

…2759) * No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes` PR Close angular#22759

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

…#22759) PR Close angular#22759

angular-automatic-lock-bot · 2019-09-13T12:52:23Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer comp: aio target: major This PR is targeted for the next major release labels Mar 14, 2018

petebacondarwin requested review from IgorMinar, gkalpak, jenniferfell and jbogarthyde March 14, 2018 09:59

googlebot added the cla: yes label Mar 14, 2018

petebacondarwin commented Mar 14, 2018

View reviewed changes

petebacondarwin added this to REVIEW in docs-infra Mar 14, 2018

gkalpak reviewed Mar 14, 2018

View reviewed changes

petebacondarwin added 7 commits March 14, 2018 19:50

build(aio): add checkContentRules processor

bd395c0

This processor will enable us to write rules about how the content should appear, such as: * no headings in markdown content * only one sentence per line * no single character parameter names * etc.

build(aio): create noMarkdownHeadings content rule

9447e94

This content rule can check what markdown headings appear in content properties.

build(aio): implement rules to prevent headings in content

87d950a

* No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes`

build(aio): create minLength content rule

29afa57

This rule can be used to ensure that properties contain a minimum number of characters.

build(aio): implement rules to prevent short parameter names

c8c3233

fixup! build(aio): create noMarkdownHeadings content rule

f5f3c51

fixup! build(aio): create noMarkdownHeadings content rule

97b8fec

petebacondarwin force-pushed the aio-api-check-headings branch from 19e89ba to 97b8fec Compare March 14, 2018 19:50

fixup! build(aio): add checkContentRules processor

d64e868

gkalpak approved these changes Mar 14, 2018

View reviewed changes

IgorMinar approved these changes Apr 12, 2018

View reviewed changes

IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 12, 2018

IgorMinar removed request for jenniferfell and jbogarthyde April 12, 2018 07:06

IgorMinar closed this in e0ae74d Apr 12, 2018

IgorMinar pushed a commit that referenced this pull request Apr 12, 2018

build(aio): create noMarkdownHeadings content rule (#22759)

7a8c581

This content rule can check what markdown headings appear in content properties. PR Close #22759

IgorMinar pushed a commit that referenced this pull request Apr 12, 2018

build(aio): implement rules to prevent headings in content (#22759)

1619160

* No headings are allowed in `description` and `shortDescription` * Only heading level 3 is allowed in `usageNotes` PR Close #22759

IgorMinar pushed a commit that referenced this pull request Apr 12, 2018

build(aio): create minLength content rule (#22759)

fa11d78

This rule can be used to ensure that properties contain a minimum number of characters. PR Close #22759

IgorMinar pushed a commit that referenced this pull request Apr 12, 2018

build(aio): implement rules to prevent short parameter names (#22759)

aae437c

PR Close #22759

petebacondarwin deleted the aio-api-check-headings branch April 12, 2018 07:13

petebacondarwin removed this from REVIEW in docs-infra Apr 12, 2018

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create noMarkdownHeadings content rule (angular#22759)

5ba597b

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create minLength content rule (angular#22759)

244b4cd

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): implement rules to prevent short parameter names (angular…

520f507

…#22759) PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create noMarkdownHeadings content rule (angular#22759)

e57e033

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create minLength content rule (angular#22759)

374a2cb

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): implement rules to prevent short parameter names (angular…

6dbd3c3

…#22759) PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create noMarkdownHeadings content rule (angular#22759)

5eaa61b

This content rule can check what markdown headings appear in content properties. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): create minLength content rule (angular#22759)

3273e26

This rule can be used to ensure that properties contain a minimum number of characters. PR Close angular#22759

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 13, 2018

build(aio): implement rules to prevent short parameter names (angular…

0670085

…#22759) PR Close angular#22759

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aio + docs/api: implement content checks #22759

aio + docs/api: implement content checks #22759

petebacondarwin commented Mar 14, 2018 •

edited

mary-poppins commented Mar 14, 2018

petebacondarwin Mar 14, 2018

mary-poppins commented Mar 14, 2018

gkalpak left a comment

gkalpak Mar 14, 2018

petebacondarwin Mar 14, 2018

gkalpak Mar 14, 2018

gkalpak Mar 14, 2018

gkalpak Mar 14, 2018

petebacondarwin Mar 14, 2018

petebacondarwin commented Mar 14, 2018 •

edited

mary-poppins commented Mar 14, 2018

petebacondarwin commented Mar 14, 2018

IgorMinar left a comment

IgorMinar Apr 12, 2018

petebacondarwin Apr 12, 2018

petebacondarwin Apr 12, 2018

petebacondarwin commented Apr 12, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

		@@ -0,0 +1,8 @@
		module.exports = function createMinLengthRule(minLength) {
		minLength = minLength \|\| 2;

aio + docs/api: implement content checks #22759

aio + docs/api: implement content checks #22759

Conversation

petebacondarwin commented Mar 14, 2018 • edited

mary-poppins commented Mar 14, 2018

Choose a reason for hiding this comment

mary-poppins commented Mar 14, 2018

gkalpak 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

petebacondarwin commented Mar 14, 2018 • edited

mary-poppins commented Mar 14, 2018

petebacondarwin commented Mar 14, 2018

IgorMinar 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

petebacondarwin commented Apr 12, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

petebacondarwin commented Mar 14, 2018 •

edited

petebacondarwin commented Mar 14, 2018 •

edited