-
Notifications
You must be signed in to change notification settings - Fork 7
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
Enhancement: Allow use of optimize
options and add recommended
config
#60
Conversation
0d1a3b6
to
4436f39
Compare
Hi @brettz9 this looks like a really thorough and well thought out improvement. I will take a closer look at it soon. Thanks for contributing! |
… at Node 6 - Enhancement: Allow use of `optimize` options - Enhancement: Add `recommended` and `all` configs which avoid need for `plugins` and `rules` together - Linting: Apply eslint:recommended, with a few additional rules and an ignore file - Travis: Update from 7 to check 8, 10, 12 - npm: Add recommended fields (bugs, homepage, contributors); use https URL for auto-opening within IDEs and linting against package.json validator - npm: Add `eslint` script - npm: Update devDeps - Maintenance: `.editorconfig` to enforce spacing on Markdown - Testing: Add nyc coverage
I've rebased on the latest changes and reconciled the |
Just a friendly reminder if you could take a look when you have a chance... |
tests/lib/rules/optimize-regex.js
Outdated
'var foo = /baz/i', | ||
'var foo = /bar/mig', | ||
'var foo = /\\/\\./', | ||
'var foo = /[/\\\\]$/', | ||
{ | ||
code: 'var re = /[0-9]/', | ||
output: 'var re = /\\d/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we backlisted charClassToMeta
, should this output be like this? shouldn't it still be var re = /[0-9]/
? maybe I'm missing something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, yes, you're right. Fixed.
I also added a test using blacklist
which was still failing.
@brettz9 this somehow slipped out of my view. I left a comment, but besides that it looks good |
@brettz9 thanks for contributing! |
optimize
optionsrecommended
andall
configs which avoid need forplugins
andrules
togetherauto-opening within IDEs and linting against package.json validator
eslint
script.editorconfig
to enforce spacing on MarkdownWhile we could avoid the breaking change by toggling to an older version of nyc in Travis for Node 6, with many projects dropping support even for Node 8 (which has reached Node end-of-life support), I think it may be easier to just drop the support (and avoid any inconsistencies in the nyc coverage results between the different nyc versions).