Skip to content

Move settings to options #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
golopot opened this issue May 12, 2019 · 10 comments · Fixed by #318
Closed

Move settings to options #216

golopot opened this issue May 12, 2019 · 10 comments · Fixed by #318

Comments

@golopot
Copy link
Collaborator

golopot commented May 12, 2019

It is hard to find out which setting adheres to which rule. It is better to move settings specific to one rule to the option of that rule.

@brettz9
Copy link
Collaborator

brettz9 commented May 14, 2019

Could you change the issue title to not be specific to require-param? I think it would be better in the case of most rules to allow for options instead of settings. (There may be a few though which interact across rules.)

@golopot
Copy link
Collaborator Author

golopot commented May 15, 2019

It seems that only require-param and check-examples have accompanied settings.

@brettz9
Copy link
Collaborator

brettz9 commented May 16, 2019

There are others; e.g., valid-types invokes utils.isNamepathType which, passes on the checkSeesForNamepaths setting, etc. It is a little harder to follow in the source because the settings are in iterateJsdoc and the rule pages typically interact with these settings through utils.

@brettz9
Copy link
Collaborator

brettz9 commented May 16, 2019

Also check-tag-names has the setting additionalTagNames which only it uses, and valid-types has two settings (allowEmptyNamepaths and checkSeesForNamepaths) that only it uses. And require-jsdoc has exemptEmptyFunctions.

require-example only has avoidExampleOnConstructors and require-returns only has forceRequireReturn.

The only setting that really is used across rules is tagNamePreference and I actually think we might change it so that it only works with check-tag-names, as the various require and param rules which use it are, because of it, failing to check for violations in non-preferred (or non-default) tags (e.g., if I prefer @param, but have a problem in @arg, it won't be caught). While this might be seen as a minor optimization if one knows one has no need to check for other synonyms, I think this is not worth it. But the option does make sense in check-tag-names.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site)
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site)
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site)
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site); affects `check-tag-names` and various param rules
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site); affects `check-tag-names` and various param rules
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site); affects `check-tag-names` and various param rules
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Fix: Remove `parameter` as accepted synonym for `@param` (not listed at site); affects `check-tag-names` and various param rules
- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Refactoring: Avoid checking for preferred name for `example` (as there are no official synonyms)
- Refactoring: Toward resolving gajus#216, group settings by which rules use them
- Refactoring: Rename `isLink` inner function to `isInlineTag` for accuracy
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…check-types should be checking borrows and namepath types; flesh out require-jsdoc including scopes and tags/aliases

- Enhancement: Have `require-hyphen-before-param-description` check `tagNamePreference` setting
- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…cking borrows and namepath types; expand valid-types tags in docs; flesh out require-jsdoc docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…cking borrows and namepath types; expand valid-types tags in docs; flesh out require-jsdoc docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 16, 2019
…cking borrows and namepath types; expand valid-types tags in docs; flesh out require-jsdoc docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…cking borrows and namepath types; expand valid-types tags in docs; flesh out require-jsdoc docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…docs including scopes and tags/aliases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…iases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags/aliases listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
- Docs: Add "Settings" column in rule tables to indicate applicable settings
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…iases

- Docs: Toward resolving gajus#216, document which rules employ each of the settings
- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags/aliases listed for rules
- Docs: Observation about `Object.create(null)` and `check-types`
- Docs: Add "Settings" column in rule tables to indicate applicable settings
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue May 17, 2019
…of the settings

- Docs: Add require-jsdoc docs per gajus#208
- Docs: Fix/add to tags/aliases listed for rules
- Docs: Add "Settings" column in rule tables to indicate applicable settings
- Docs: Observation about `Object.create(null)` and `check-types`
- Refactoring: Avoid nested `if`
@golopot golopot changed the title Move settings of require-param from plugin settings to rule option Move settings to options Jun 20, 2019
@golopot
Copy link
Collaborator Author

golopot commented Jun 20, 2019

What is the process for moving settings to options? I suggest we do it one-step in a major release: add options, stop reading settings, and warns to stderr report an error in case a removed setting is set.

@brettz9
Copy link
Collaborator

brettz9 commented Jun 22, 2019

Sounds like a fine approach to me... I have my hands pretty full now, but a PR would be welcome...

Bear in mind though re: breaking changes... As much as one doesn't wish to make breaking changes in any project, in a linting project, it can be as little total pain for a project to keep up with more frequent breaking changes and make a few incremental changes as it is to "rip off the bandage" and do it all at once, and with linting, breaking changes are far more likely (especially if one were to interpret every minor loosening or tightening as a breaking change).

With linting, particularly ESLint, one has good feedback about what is failing and why, so it doesn't generally take a lot of debugging and deep refactoring work that might be better done all at once as can be the case for non-linting API changes.

But doing it all at once does make sense here, especially as the changes are related, and it is better to increase consistency with the API.

@golopot
Copy link
Collaborator Author

golopot commented Jul 7, 2019

It will be great if someone can take up remaining part of this issue, that is, move settings of check-examples to options.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 7, 2019

I've started a branch to do so; just need to fix up the docs.

We might then see about removing the already-deprecated allowOverrideWithoutParam, allowImplementsWithoutParam, and allowAugmentsExtendsWithoutParam as well as changing the default of their replacements to true.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 7, 2019
BREAKING CHANGE:

The following settings have been removed and became options
under `check-examples`

* `settings.jsdoc.exampleCodeRegex`
* `settings.jsdoc.rejectExampleCodeRegex`
* `settings.jsdoc.matchingFileName`
* `settings.jsdoc.baseConfig`
* `settings.jsdoc.configFile`
* `settings.jsdoc.eslintrcForExamples`
* `settings.jsdoc.allowInlineConfig`
* `settings.jsdoc.reportUnusedDisableDirectives`
* `settings.jsdoc.captionRequired`
* `settings.jsdoc.noDefaultExampleRules`
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 7, 2019
BREAKING CHANGE:

The following settings have been removed and became options
under `check-examples`

* `settings.jsdoc.exampleCodeRegex`
* `settings.jsdoc.rejectExampleCodeRegex`
* `settings.jsdoc.matchingFileName`
* `settings.jsdoc.baseConfig`
* `settings.jsdoc.configFile`
* `settings.jsdoc.eslintrcForExamples`
* `settings.jsdoc.allowInlineConfig`
* `settings.jsdoc.reportUnusedDisableDirectives`
* `settings.jsdoc.captionRequired`
* `settings.jsdoc.noDefaultExampleRules`
@gajus
Copy link
Owner

gajus commented Jul 7, 2019

🎉 This issue has been resolved in version 13.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Jul 7, 2019
brettz9 added a commit to interfaced/eslint-plugin-jsdoc that referenced this issue Jul 7, 2019
* master:
  feat(`require-param`): remove deprecated settings `allowOverrideWithoutParam`, `allowImplementsWithoutParam`, `allowAugmentsExtendsWithoutParam`
  feat(`check-examples`): move settings to options (fixes gajus#216)
@gajus
Copy link
Owner

gajus commented Sep 1, 2019

🎉 This issue has been resolved in version 15.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants