Skip to content

Update: deprecate no-invalid-* rules and add valid-* rules (fixes #103) #132

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

Merged
merged 3 commits into from
Aug 8, 2017

Conversation

mysticatea
Copy link
Member

@mysticatea mysticatea commented Aug 5, 2017

Fixes #103.

This PR renames no-invalid-* rules to avoid double negation.
New naming is valid-* similar to some core rules; valid-jsdoc and valid-typeof.

This PR deprecates no-invalid-* rules and doesn't remove those. So this is not a breaking change. We can remove those in future.

Additionally, this PR creates the table of deprecated rules and removes deprecated rules from rule tables.

@mysticatea
Copy link
Member Author

For reference, new rules are added by the script below:

const fs = require('fs')
const path = require('path')

for (const root of ['docs/rules', 'lib/rules', 'tests/lib/rules']) {
  for (const filename of fs.readdirSync(root)) {
    if (!filename.startsWith('no-invalid-')) {
      continue
    }
    const src = path.join(root, filename)
    const dst = path.join(root, 'valid-' + filename.slice('no-invalid-'.length))
    const content = fs.readFileSync(src, 'utf8')
      .replace(/no-invalid-/g, 'valid-')
      .replace(/disallow invalid/g, 'enforce valid')
      .replace(/Disallow invalid/g, 'Enforce valid')

    fs.writeFileSync(dst, content)
  }
}

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

LGTM 👍 two minor copy suggestions

### Deprecated

> - :warning: We are going to remove deprecated rules in future. Please migrate to its replaced rules.
> - :innocent: We don't fix bugs which are in deprecated rules since we don't have enough resource.
Copy link
Member

Choose a reason for hiding this comment

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

resources

`).join('\n') + `
### Deprecated

> - :warning: We are going to remove deprecated rules in future. Please migrate to its replaced rules.
Copy link
Member

Choose a reason for hiding this comment

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

We're going to remove deprecated rules in the next major release. Please migrate to successor/new rules.

@mysticatea
Copy link
Member Author

Thank you for review!

I updated this PR.

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

Successfully merging this pull request may close these issues.

2 participants