Skip to content
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

Normalize rule config values #169

Merged
merged 2 commits into from
Apr 28, 2021
Merged

Normalize rule config values #169

merged 2 commits into from
Apr 28, 2021

Conversation

rekmarks
Copy link
Member

The rule validation script wasn't taking into account that extended configs can use numerical (0, 1, 2) or text (off, warn, error) notation for rule configs. This PR normalizes all rules in the snapshot to use text notation.

I had to parameterize this behavior to make an exception for when we compute the required Prettier rules, because Prettier intentionally uses numerical notation for a subset of its rules to distinguish them from the rest.

Applying this change caught some redundantly configured rules.

@rekmarks rekmarks requested a review from a team as a code owner April 19, 2021 20:50
).reduce((allRules, [ruleName, ruleValue]) => {
// Rules set to 'off' should never be enabled.
// Rules set to 0 (number) may sometimes be included. We don't attend to those.
// https://github.com/Prettier/eslint-config-Prettier/blob/abf3ba1/index.js#L7-L9
Copy link
Member

Choose a reason for hiding this comment

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

Huh, interesting way of denoting optional settings.

Gudahtt
Gudahtt previously approved these changes Apr 20, 2021
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM - great catch!

scripts/validate-configs.js Outdated Show resolved Hide resolved
@rekmarks rekmarks merged commit 8646767 into main Apr 28, 2021
@rekmarks rekmarks deleted the normalize-snapshot-rule-values branch April 28, 2021 20:41
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.

2 participants