chore: enable eslint-plugin-jsdoc

[pkuczynski]

I added all rules, as recommended set only turns them as warnings, while I guess in the future we would like to keep them as errors?

[ST-DDT]

Why do we need @file? Isn't it enough to just add jsdocs to the class itself?
Our files only contain one class anyway.

[pkuczynski]

Why do we need @file? Isn't it enough to just add jsdocs to the class itself?
Our files only contain one class anyway.

I was hoping it would enforce the jsdoc on class (see #351 (comment)), but it does not. @brettz9 is it possible to do it?

[ST-DDT]

IMO we should stick to the default configuration as much as possible and only change the options we actually want to change or have to change to avoid lint errors during migration. Maybe with a hint, that its temporary.
AFAICT our code doesn't use return, yield or throws and thus we don't have to change the config, unless we want to explicitly turn these rules on.

[pkuczynski]

IMO we should stick to the default configuration as much as possible and only change the options we actually want to change or have to change to avoid lint errors during migration. Maybe with a hint, that its temporary.
AFAICT our code doesn't use return, yield or throws and thus we don't have to change the config, unless we want to explicitly turn these rules on.

That's what I actually did. I only adjusted the default setup to reflect ours. The only reason why I put all the rules explicitly was for the future so we can change from warn to error, as recommended set have everything set as a warning...

[ST-DDT]

Why do we need @file? Isn't it enough to just add jsdocs to the class itself?
Our files only contain one class anyway.

I was hoping it would enforce the jsdoc on class (see #351 (comment)), but it does not. brettz9 is it possible to do it?

How about https://github.com/gajus/eslint-plugin-jsdoc#require-jsdoc

• require
  • ClassDeclaration: true

[pkuczynski]

• require
  • ClassDeclaration: true

Works great! Thanks for helping out. Please re-review.

[Shinigami92]

That's a very verbose configuration 😕
I would like to say out my veto for now and would like to help this evening to try to find a more minimal/recommended-based config setup

[brettz9]

That's a very verbose configuration 😕 I would like to say out my veto for now and would like to help this evening to try to find a more minimal/recommended-based config setup

Feel free to submit issues or well-justified PRs to add to the configs (e.g., an identical warnings based config). Though I'm pretty occupied, it is in fact a good time for me to be revisiting our configs at least for decision-making. I'd like to actually switch the default type mode to typescript, for example, if there is community support in favor.

[Shinigami92]

Feel free to submit issues or well-justified PRs to add to the configs (e.g., an identical warnings based config). Though I'm pretty occupied, it is in fact a good time for me to be revisiting our configs at least for decision-making. I'd like to actually switch the default type mode to typescript, for example, if there is community support in favor.

I think it would be a good idea to have some opinionated configurations
This was so far my personal config https://github.com/prettier/plugin-pug/blob/11640677585d45211387bfebadcaec36d28c5049/.eslintrc.js#L87-L119
But I'm not sure if it's up to date right now 🙂

Beside that, I think recommended set rules to warn could be a good idea, because mostly JSDoc will not break the code anyways but just are stylistic suggestions
Only stuff like "no types in @param for TypeScript" should throw errors

[ST-DDT]

Beside that, I think recommended set rules to warn could be a good idea, because mostly JSDoc will not break the code anyways but just are stylistic suggestions

And we get a warning in the github review too, so this might be sufficient.

Only stuff like "no types in @param for TypeScript" should throw errors

AFAIK we don't write types there!?

[brettz9]

Beside that, I think recommended set rules to warn could be a good idea, because mostly JSDoc will not break the code anyways but just are stylistic suggestions

And we get a warning in the github review too, so this might be sufficient.

For eslint-plugin-jsdoc, I think it is good to have both as separate configs because:

1. Some take the docs seriously enough to want to ensure it is always accurate
2. It technically could break the code to have bad TypeScript types in JSDoc (though admittedly presumably people will be using TypeScript to do this instead of our JSDoc type parser for heavier checking anyways).

Only stuff like "no types in @param for TypeScript" should throw errors

AFAIK we don't write types there!?

I think he or she is speaking to me about a preference for eslint-plugin-jsdoc to generally only warn, but we have a rule for that in case someone accidentally does add types.

[ST-DDT]

@pkuczynski Please change the configuration so that it requires a single newline between the params block and @examples to make it easier to visually distinguish. Or what do you think?

[brettz9]

@pkuczynski Please change the configuration so that it requires a single newline between the params block and @examples to make it easier to visually distinguish. Or what do you think?

Note for our jsdoc/tag-lines rule, we currently don't support configuration to specify in terms of lines before a certain tag. It's currently in the context of 0+ lines after each instance of any given tag (though allowing for end lines to be dropped).

[pkuczynski]

This was so far my personal config prettier/plugin-pug@1164067/.eslintrc.js#L87-L119 But I'm not sure if it's up to date right now 🙂

Do you want me to add the jsdoc/require-jsdoc settings like you have in plugin-pug?

Beside that, I think recommended set rules to warn could be a good idea, because mostly JSDoc will not break the code anyways but just are stylistic suggestions Only stuff like "no types in @param for TypeScript" should throw errors

I think once we clear the current jsdoc, we should make those rules to fail the build, as warnings are only shown for reviewers and not for the PR author. It would save us a lot of time as reviewers if PR authors would get quick and solid feedback from the automation tools.

Documentation is an important part of faker and I believe we should keep a strong guard on it.

[pkuczynski]

I think he or she is speaking to me about a preference for eslint-plugin-jsdoc to generally only warn, but we have a rule for that in case someone accidentally does add types.

require-param-type only allow to force param type, but I can't see a way to prevent it from being added @brettz9?

[pkuczynski]

@pkuczynski Please change the configuration so that it requires a single newline between the params block and @examples to make it easier to visually distinguish. Or what do you think?

I tried, but I don't think that's possible, unfortunately?

[brettz9]

I think he or she is speaking to me about a preference for eslint-plugin-jsdoc to generally only warn, but we have a rule for that in case someone accidentally does add types.

require-param-type only allow to force param type, but I can't see a way to prevent it from being added @brettz9?

jsdoc/no-types.

@pkuczynski Please change the configuration so that it requires a single newline between the params block and @examples to make it easier to visually distinguish. Or what do you think?

I tried, but I don't think that's possible, unfortunately?

Right, not at this time. You can subscribe to gajus/eslint-plugin-jsdoc#782 .

[pkuczynski]

jsdoc/no-types.

omg! I am blind, thx!

Right, not at this time. You can subscribe to gajus/eslint-plugin-jsdoc#782 .

subscribed, thx!

[codecov]

Codecov Report

Merging #400 (89fcec1) into main (271c400) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #400   +/-   ##
=======================================
  Coverage   94.24%   94.24%           
=======================================
  Files        1920     1920           
  Lines      173955   173955           
  Branches      108      108           
=======================================
  Hits       163942   163942           
  Misses       9905     9905           
  Partials      108      108           

[Shinigami92]

We can merge this now or can wait on the sort-tags rule

[ST-DDT]

We can merge this now or can wait on the sort-tags rule

I would probably merge it now, since we don't have an ETA for the rule to be added.