Skip to content

Conversation

@lznakano
Copy link
Contributor

@lznakano lznakano commented Sep 7, 2016

Please take a look at .eslintrc file. It lists a set of rules. Only parameter is required, the rest rules are set to warning (0:off, 1:warning, 2:true).

"jsdoc/require-param-description": 1,
"jsdoc/require-param-type": 1,
"jsdoc/require-returns-description": 1,
"jsdoc/require-returns-type": 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to relax the rules a bit, otherwise we'll have too many warnings, and people will not pay attention to real problems.

I suggest the following defaults:
"jsdoc/check-param-names": 2,
"jsdoc/check-tag-names": 1,
"jsdoc/check-types": 1,
"jsdoc/newline-after-description": 0,
"jsdoc/require-description-complete-sentence": 0,
"jsdoc/require-hyphen-before-param-description": 0,
"jsdoc/require-param": 2,
"jsdoc/require-param-description": 0,
"jsdoc/require-param-type": 0,
"jsdoc/require-returns-description": 0,
"jsdoc/require-returns-type": 1

We would still want to preserve those rules in the file, so that people who work on API documentation can turn them on to make docs consistent.

Reference link: https://github.com/gajus/eslint-plugin-jsdoc

@tgoldina
Copy link
Contributor

tgoldina commented Sep 8, 2016

Other than the defaults everything looks great.

I noticed that LayoutCntlr.js is failing because the following function signature format is unsupported by eslint-plugin-jsdoc:
dispatchShowDropDown({view}={}).

Unfortunately, the rules that require function signature parsing are the most useful:
"jsdoc/check-param-names": 2,
"jsdoc/require-param": 2,

@tgoldina
Copy link
Contributor

tgoldina commented Sep 8, 2016

Review complete.

As Loi said, we can fix LayoutCntlr.js
dispatchShowDropDown({view}={}) -> dispatchShowDropDown({view})
and move on.

@lznakano lznakano merged commit 6d92bb9 into dev Sep 8, 2016
@robyww robyww deleted the DM-7508-eslintRules branch September 28, 2016 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants