match-description options #294

brettz9 · 2019-06-25T02:09:55Z

BREAKING CHANGE(require-description): remove noDefaults option and change contexts to always override defaults
feat(match-description): allow mainDescription: string|boolean to override or disable main description separate from default
feat(match-description): add contexts options for control on which contexts the rules apply
feat(match-description): report line number with match-description
feat(match-description): allow reporting multiple errors when main description validation fails
feat(require-jsdoc): add contexts option to allow working with any node type
fix(match-description, require-description): allow contexts to work with any node type
fix(match-description): tighten default regex to require punctuation at the end even if only a single character

test/rules/assertions/matchDescription.js

src/rules/matchDescription.js

golopot · 2019-06-25T04:15:25Z

src/rules/matchDescription.js

          matchDescription: {
            format: 'regex',
            type: 'string'
          },
+          noDefaults: {


Why this option? Isn't it more intuitive to let provided contexts to override the default context?

It is extra work for users. I believe people will almost always want to keep the defaults. Also, that is the existing behavior for require-description.

Why this option? Isn't it more intuitive to let provided contexts to override the default context?

Agreed. Otherwise, I need to be aware of what the defaults are. contexts should just override the default.

There have been a few rules already added using this approach, e.g., require-description, check-types. Revert them as well?

(check-types is somewhat different in referring to avoid the defaults for your preferred types, while require-description is more like as in this PR.)

I went ahead and added a commit to make a breaking change to remove noDefaults from require-description (and for match-description) though I haven't done so for check-types unless you want to confirm. Let me know if that is ok.

@gajus : Since your approval kind of overlapped with my reversion, I just wanted to confirm--are you happy with my breaking change to undo require-description's use of noDefaults (in addition to removing it in match-description)?

golopot

I would prefer not providing the option contexts, why would you want to include or exclude some node type?

.README/rules/require-jsdoc.md

brettz9 · 2019-06-25T04:35:51Z

contexts is very useful as one can, as per the request in #293 , ensure that one has valid descriptions, e.g., above properties--not only the default behavior of matching descriptions on functions only.

golopot · 2019-06-25T04:38:31Z

I mean the rule should apply to all jsdoc without exception (#214).

brettz9 · 2019-06-25T04:42:10Z

I think it is better to be under user control, whether for performance or just because projects differ from one another in their needs; but if you want to expand the defaults, I can change to support all node types by default.

src/rules/matchDescription.js

.README/rules/match-description.md

gajus · 2019-06-25T06:45:43Z

src/rules/matchDescription.js

          matchDescription: {
            format: 'regex',
            type: 'string'
          },
+          noDefaults: {


Why this option? Isn't it more intuitive to let provided contexts to override the default context?

Agreed. Otherwise, I need to be aware of what the defaults are. contexts should just override the default.

golopot · 2019-06-25T13:53:44Z

In the future please keep unrelated changes in different pull requests. Stuffing all changes in a pr 1) makes it much harder to review, 2) make the reviewer unable to accept one part of change and reject another.

brettz9 · 2019-06-26T11:59:01Z

This PR has now been narrowed somewhat, and I think it is ready for merging, @gajus , if you could respond to my comments ending at #294 (comment) mentioning how I added a breaking change that I understood you wanted around the time you approved this PR, so I wanted to confirm that breaking change was indeed ok.

golopot

LGTM.

Would you like to rebase the commits? The commit history of master can be less noisy. Also allows sematic-release to generate proper release notes.

brettz9 · 2019-06-26T12:11:15Z

If y'all are ready for rebasing, sure.

src/rules/requireJsdoc.js

…ple errors when main description validation fails

…at the end even if only a single character

…node type

… with any node type

…rride or disable main description separate from default

…change `contexts` to always override defaults

…`ClassExpression` and `ObjectExpression`

…uire-jsdoc); docs, schema

gajus · 2019-06-27T12:20:35Z

🎉 This PR is included in version 9.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

brettz9 mentioned this pull request Jun 25, 2019

Validation for description-like comments? #293

Closed

brettz9 requested a review from golopot June 25, 2019 02:48

golopot reviewed Jun 25, 2019

View reviewed changes

test/rules/assertions/matchDescription.js Outdated Show resolved Hide resolved

golopot reviewed Jun 25, 2019

View reviewed changes

src/rules/matchDescription.js Show resolved Hide resolved

golopot reviewed Jun 25, 2019

View reviewed changes

golopot requested changes Jun 25, 2019

View reviewed changes

golopot reviewed Jun 25, 2019

View reviewed changes

.README/rules/require-jsdoc.md Outdated Show resolved Hide resolved

gajus requested changes Jun 25, 2019

View reviewed changes

gajus approved these changes Jun 25, 2019

View reviewed changes

brettz9 mentioned this pull request Jun 25, 2019

Except for return/param rules, most rules should not be limited to functions #214

Closed

brettz9 mentioned this pull request Jun 26, 2019

Fix(eslint): Add object expression in getJSDocComment #295

Merged

golopot approved these changes Jun 26, 2019

View reviewed changes

golopot reviewed Jun 27, 2019

View reviewed changes

src/rules/requireJsdoc.js Show resolved Hide resolved

brettz9 force-pushed the match-description-options branch 2 times, most recently from d718c34 to 91344a4 Compare June 27, 2019 10:58

brettz9 added 9 commits June 27, 2019 19:35

feat(match-description): report line number and allow reporting multi…

3b61dcc

…ple errors when main description validation fails

fix(match-description): tighten default regex to require punctuation …

58ca186

…at the end even if only a single character

feat(require-jsdoc): add contexts option to allow working with any …

3e0775a

…node type

fix(match-description, require-description): allow contexts to work…

299b095

… with any node type

feat(match-description): allow mainDescription: string|false to ove…

11cf9f5

…rride or disable main description separate from default

BREAKING CHANGE(require-description): remove noDefaults option and …

67e8c5d

…change `contexts` to always override defaults

testing(match-description): Add tests for contexts with parents of …

baa166a

…`ClassExpression` and `ObjectExpression`

Remove noDefaults from match-description, require-jsdoc

1fb8bd5

Change contexts to always override defaults (match-description, req…

2c34e21

…uire-jsdoc); docs, schema

brettz9 force-pushed the match-description-options branch from 91344a4 to 2c34e21 Compare June 27, 2019 12:03

docs: generate docs

f075b67

brettz9 merged commit ecc27c5 into gajus:master Jun 27, 2019

gajus added the released label Jun 27, 2019

brettz9 deleted the match-description-options branch June 27, 2019 13:01

Uh oh!

match-description options #294

match-description options #294

Uh oh!

Conversation

brettz9 commented Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

golopot Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettz9 Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

gajus Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

brettz9 Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

brettz9 Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

brettz9 Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

brettz9 Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

golopot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brettz9 commented Jun 25, 2019

Uh oh!

golopot commented Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brettz9 commented Jun 25, 2019

Uh oh!

Uh oh!

Uh oh!

gajus Jun 25, 2019

Choose a reason for hiding this comment

Uh oh!

golopot commented Jun 25, 2019

Uh oh!

brettz9 commented Jun 26, 2019

Uh oh!

golopot left a comment

Choose a reason for hiding this comment

Uh oh!

brettz9 commented Jun 26, 2019

Uh oh!

Uh oh!

gajus commented Jun 27, 2019

Uh oh!

Uh oh!

brettz9 commented Jun 25, 2019 •

edited

Loading

golopot Jun 25, 2019 •

edited

Loading

golopot commented Jun 25, 2019 •

edited

Loading