Skip to content

Conversation

@Marsup
Copy link
Contributor

@Marsup Marsup commented Sep 19, 2014

Hello,

As discussed in outmoded/discuss#22, I'm trying to implement linting in lab.
This is a WIP and shouldn't be merged right now as tests are not even done.
So far I only have support for eslint, I included @geek's rules but lab itself is full of errors so it'll need tweaking.
It will automatically detect eslint settings at the root of the project and if none use what should be our default rules.
The possibility to have multiple linters is deliberate and probably wanted.
Console output was faster to implement but others will follow.
I'd like feedback about my approach.

Thanks.

@hueniverse hueniverse added the feature New functionality or improvement label Sep 19, 2014
bin/_lab Outdated
Copy link
Member

Choose a reason for hiding this comment

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

else on newline

@geek
Copy link
Member

geek commented Sep 24, 2014

I like this approach, be sure to add more tests and document the option in the readme.

@Marsup
Copy link
Contributor Author

Marsup commented Oct 2, 2014

Updated the PR with these suggestions. I've actually spent a lot of time trying to find the correct settings to validate lab itself. If you have better rules before starting writing our own, I'll take em :)

@geek
Copy link
Member

geek commented Oct 2, 2014

@Marsup thanks, I'll merge this after 4.5.2. Maybe we should start with only a few rules?

@geek geek added this to the 4.6.0 milestone Oct 2, 2014
@geek geek self-assigned this Oct 2, 2014
@geek
Copy link
Member

geek commented Oct 2, 2014

@Marsup any chance that you can add some tests for the new option?

@Marsup
Copy link
Contributor Author

Marsup commented Oct 2, 2014

Sure, I'll hopefully complete coverage in a few days, just a bit busy currently.

@Marsup
Copy link
Contributor Author

Marsup commented Oct 5, 2014

@geek Coverage is good now, linting doesn't make a test run fail though, but at least it's usable until we manage to find rules that fit the projects. Travis's output is weird, a red cross but no error, I don't get it.

geek added a commit that referenced this pull request Oct 6, 2014
WIP - Add linting support
@geek geek merged commit d955c60 into hapijs:master Oct 6, 2014
@geek
Copy link
Member

geek commented Oct 6, 2014

Thanks for this, it will be a huge help

@Marsup Marsup deleted the lint branch October 7, 2014 20:58
mdarnall added a commit to mdarnall/joi that referenced this pull request Jan 21, 2015
* based on last activity on Lab (hapijs/lab#213)
* make all rules that are best practices, but not in styleguide as
warnings
@lock
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.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants