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

Editor support improvements for Atom, Sublime, and others: #2575

Closed
wants to merge 1 commit into from

Conversation

@garthk
Copy link

garthk commented May 31, 2015

npm test uses lab to enforce the code style. We can help contributors by also having their editor flag the problems as they type. This PR adds:

  • Local eslint so lint plugins can report on divergence from Hapi JavaScript style
  • .editorconfig so editorconfig plugins can apply Hapi indentation and line termination

I copied the .eslintrc from lab, and adjusted the plugins entry to make Atom happy.

Hazard: need to track lab changes to .eslintrc and its package.json dependencies on eslint and eslint-plugin-hapi.

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented May 31, 2015

ESlint now offers the possibility to include another rc, I'd prefer that.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 31, 2015

Using the extends option from eslint would indeed be way better, maybe worth it to separate config in it's own module?

@garthk

This comment has been minimized.

Copy link
Author

garthk commented May 31, 2015

I reckon extends is a great idea, and the example in the docs is pushing the rules into their own module. Win!

Blocking problems:

  • lab takes a dependency on eslint@0.19.x, which doesn't support extends (fix: hapijs/lab#366)
  • lab's version of .eslintrc needs its plugins prefix removed
  • the Atom linter-eslint package doesn't seem to respect extends, even when using a local eslint
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 31, 2015

PR should be done later today, needs a small eslint version bump.
The config should be revised anyway when making e separate module.
And I suggest creating an issue there, I've tried Atom recently and bumped
into the same issue so getting that fixed would be great.

On Sun, 31 May 2015 11:34 Garth Kidd notifications@github.com wrote:

I reckon extends
http://eslint.org/docs/user-guide/configuring.html#extending-configuration-files
is a great idea, and the example in the docs is pushing the rules into
their own module. Win!

Blocking problems:


Reply to this email directly or view it on GitHub
#2575 (comment).

Met vriendelijke groeten

Adri Van Houdt

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented May 31, 2015

Yet another module, not sure of the benefit.

@garthk

This comment has been minimized.

Copy link
Author

garthk commented May 31, 2015

hapi delegates lint rules to lab, so I think we already get a tick for "own module" for the time being. Can we make changing which module a separate issue, @AdriVanHoudt?

@garthk

This comment has been minimized.

Copy link
Author

garthk commented May 31, 2015

I've revised the .eslintrc and .editorconfig. They'll work when:

  • @AdriVanHoudt lands hapijs/lab#366, or lab otherwise upgrades its eslint
  • someone removes the eslint-plugin- prefix from the hapi entry in lab/lib/linters/eslint/.eslintrc

Anything else we need?

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 31, 2015

@Marsup there is https://github.com/cjihrig/eslint-plugin-hapi so that's already semi it's own module
@garthk sure
and removing the prefix doesn't break stuff?

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 31, 2015

PR done, but needs revision

* Local eslint so editors can report on divergence from Hapi JavaScript style
* .editorconfig so editors can apply Hapi indentation and line termination
@garthk garthk force-pushed the garthk:editor-friendliness branch from 2f69bfe to 5c475ec Jun 1, 2015
@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Jul 1, 2015

I'm going to pass on this for now.

@hueniverse hueniverse closed this Jul 1, 2015
@hueniverse hueniverse self-assigned this Jul 1, 2015
@lock

This comment has been minimized.

Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.