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

Eslint Integration #310

Closed
wants to merge 3 commits into from
Closed

Conversation

tusharmath
Copy link

No description provided.

This was referenced Apr 16, 2016
@tusharmath
Copy link
Author

@lorenwest Could you please take a look at the errors — https://travis-ci.org/lorenwest/node-config/jobs/123596668

I have added all the configs as per my understanding. I would need your inputs to figure out what should be changed in the code vs config.

@lorenwest
Copy link
Collaborator

Yes, I see the lint errors. Anyone want to take on the work of cleaning these up? You'll be listed in high regard as a contributor on our README page :)

@antony
Copy link

antony commented Aug 25, 2016

For better interoperability, try the eslint based, zero configuration 'standard' library instead of raw eslint. It avoids bikeshedding discussions about preferred styles.

The even better part about this, is that you can auto-format for 'standard' in most editors, avoiding a lot of manual re-formatting work.

If this is interesting, I'd probably have time to do a quick PR

@markstos
Copy link
Collaborator

@antony see the PRs #305 and #307 which already started work on being standard compliant. It seems this PR is a third iteration of that work. I have not reviewed what the remaining issues were recently, but they should be found somewhere in these 3 pull requests.

@antony
Copy link

antony commented Aug 25, 2016

Ah, interesting. So ESLint is being preferred to standard. That's a shame, as it allows configuration and personal preference which results in endless, resolution-less debates. However! The people have spoken :)

@markstos
Copy link
Collaborator

As a maintainer, I'm in favor of using eslint. I just assume to follow the "AirBnB" rules to address @antony point about debates about all the details. I also think "Pretty" may be a better choice for code formatting, while Eslint is better suited for code syntax issues.

Also, I think make eslint should be run as part of npm test

I would be interested in PR that adds Eslint inegration along with a commit that addresses all the eslint violations that are triggered when adopting it.

@jdmarshall
Copy link
Contributor

#667 Fixes a handful of these issues.

@markstos
Copy link
Collaborator

Closing due to lack of volunteer to fix the linting errors in the past 5 years.

I like the idea of using prettier for code formatting issues and eslint for code-style issues, following the AirBnB rules. An updated attempt to introduce prettier or eslint along with resolving the errors the flag is welcome.

@markstos markstos closed this Jan 16, 2022
@jdmarshall
Copy link
Contributor

The changes I made in my first PR were based on Webstorm complaints. I haven't been keeping track of their release notes so at this point I couldn't say whether they're using third party code or something they invented.

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.

None yet

5 participants