Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Fix behavior of jsRules:true to include explicitly disabled rules. #4517

Merged
merged 1 commit into from
Feb 23, 2019
Merged

Fix behavior of jsRules:true to include explicitly disabled rules. #4517

merged 1 commit into from
Feb 23, 2019

Conversation

UselessPickles
Copy link
Contributor

@UselessPickles UselessPickles commented Feb 14, 2019

PR checklist

Overview of change:

Changed behavior of jsRules:true to copy over ALL rule configurations that are compatible with JS, including any that are explicitly disabled.

Is there anything you'd like reviewers to focus on?

The original PR (#3641) that added jsRules:true functionality seemed to make a point to describe its desired/implemented behavior to copy over all "active" rules that are valid for JS. Does the term "active" mean something special that is not simply "those that are not disabled"? Is there still some kind of rule configurations that I should be skipping over?

CHANGELOG.md entry:

[bugfix] Explicit disabling of rules is now copied over to jsRules when using jsRules:true

@@ -151,7 +152,9 @@ describe("Configuration", () => {
rawConfig = {
jsRules: true,
rules: {
eofline: true,
// valid rule for JS, disabled (should be copied over)
eofline: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tweaked this existing test case to cover the new behavior of keeping disabled rules

@@ -31,7 +31,7 @@ configure which rules get run and each of their options.
- Legacy: A boolean value may be specified instead of the above object, and is equivalent to setting no options with default severity.
- Any rules specified in this block will override those configured in any base configuration being extended.
- [Check out the full rules list here][3].
- `jsRules?: any | boolean`: Same format as `rules` or explicit `true` to copy all valid active rules from rules. These rules are applied to `.js` and `.jsx` files.
- `jsRules?: any | boolean`: Same format as `rules` or explicit `true` to copy all rule configurations for JS-compatible rules from `rules`. These rules are applied to `.js` and `.jsx` files.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation is clarified

@UselessPickles
Copy link
Contributor Author

Am I supposed to add an entry to CHANGELOG.md as part of this PR?

@adidahiya adidahiya self-assigned this Feb 14, 2019
@adidahiya
Copy link
Contributor

@UselessPickles nope, those entries get autogenerated by the release script

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

Successfully merging this pull request may close these issues.

2 participants