Skip to content

Update for eslint@>2.x#13

Closed
BurtHarris wants to merge 2 commits intogulpjs:masterfrom
BurtHarris:workitem_10
Closed

Update for eslint@>2.x#13
BurtHarris wants to merge 2 commits intogulpjs:masterfrom
BurtHarris:workitem_10

Conversation

@BurtHarris
Copy link
Copy Markdown

Replace space-after-keywords with keyword-spacing
Adds eslint as a peer dependency

fixes #10

Replace space-after-keywords with keyword-spacing
Adds eslint as a peer dependency

fixes #10

Update for eslint@^2

Added peer dependency

Fixes #10
@phated
Copy link
Copy Markdown
Member

phated commented Jul 30, 2017

I don't think we needs peerDeps

@BurtHarris
Copy link
Copy Markdown
Author

BurtHarris commented Jul 30, 2017

You are probably right, I was thinking that would cause dependent projects to generate a warning if they are still using eslint@1, but testing it out that doesn't seem to work, at least with npm@3.10.10.

Update: No, I take that back. I get the following message if the update isn't done in a dependency:

npm WARN eslint-config-gulp@2.1.0 requires a peer of eslint@^2.13.1 but none was installed.

That's what I was expecting.

per @phated's feedback
Doesn't seem to hurt if using eslint@1 or eslint@2
@BurtHarris
Copy link
Copy Markdown
Author

@phated I've removed the peer dependency. Doesn't seem to hurt anything without it.

@pdehaan
Copy link
Copy Markdown
Contributor

pdehaan commented Jul 30, 2017

Yeah, peerDependencies were deprecated in npm@3 (per http://blog.npmjs.org/post/110924823920/npm-weekly-5)

@BurtHarris
Copy link
Copy Markdown
Author

BurtHarris commented Jul 30, 2017

@pdehaan, I think peerDependencies had their behavior changed in npm@3, but not deprecated. The behavior I expected is exactly what it says in that blog entry:

We will also be changing the behavior of peerDependencies in npm@3. We won’t be automatically downloading the peer dependency anymore. Instead, we’ll warn you if the peer dependency isn’t already installed. This requires you to resolve peerDependency conflicts yourself, manually, but in the long run this should make it less likely that you’ll end up in a tricky spot with your packages’ dependencies.

peerDependencies are in use in several well-maintained projects I've worked with, for example gulp-tsb has a peerDependency on the TypeScript transpiler. This way a warning is generated if TypeScript isn't also part of the same project that gulp-tsb is. But gulp-tsb doesn't get it's own copy, it uses the version the containing project uses.

@BurtHarris
Copy link
Copy Markdown
Author

BurtHarris commented Aug 3, 2017

Bump. Blocking gulpjs/vinyl-fs#270

@BurtHarris BurtHarris changed the title Update for eslint@^2.x Update for eslint@>2.x Aug 3, 2017
@phated
Copy link
Copy Markdown
Member

phated commented Aug 3, 2017

Still considering this for eslint@2 support. I'm going to take a look at this stuff with #8

GaryJones added a commit to craigsimps/gulp-wp-toolkit that referenced this pull request Aug 23, 2017
The `gulp self` tasks couldn't be run without a Gulpfile present, and a local dependency on Gulp itself.

Instead, we move to use npm scripts, and the individual tools themselves (unwrapped from gulp).

There are three self-linting tasks:

- `jsonlint` - which validates some JSON files. More files can be added as needed.
- `esvalidate` - which ensures JavaScript files are valid. The output doesn't appear to show the file name, and the suggestion at duereg/esvalidate#3 doesn't appear to work for me.
-  `eslint` - checks for good JavaScript coding practices and formatting. Rather than sticking with `eslint-config-wordpress` (which checks JS which will run in the browser), this uses the `eslint-config-gulp` config.

These can be run with, for instance, `npm run eslint`. All of the self-linting tasks can be in one go with `npm run lint`. `npm test` exists as a convenience too.

The eslintConfig in `package.json` avoids the need to have a separate `.eslintrc` file. We extend the `eslint-config-gulp` config, and specify we're using ES6.

The rules section is needed to avoid false positives regarding a change of rule name. These rules can be fixed as and when gulpjs/eslint-config-gulp#13 gets merged.

See #25.
@phated
Copy link
Copy Markdown
Member

phated commented May 14, 2018

I've opted for #15 - thanks though @BurtHarris

@phated phated closed this May 14, 2018
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.

space-after-keywords is deprecated since ESLint v2.0

4 participants