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

Update eslint to 1.0.x #412

Merged
merged 2 commits into from Aug 3, 2015

Conversation

@gergoerdosi
Copy link
Contributor

gergoerdosi commented Aug 2, 2015

In eslint 1.0.0 all rules are off by default. So it's possible that we will need to add some other rules to .eslintrc or use eslint:recommended.

http://eslint.org/docs/user-guide/migrating-to-1.0.0

Gergo Erdosi
@gergoerdosi

This comment has been minimized.

Copy link
Contributor Author

gergoerdosi commented Aug 2, 2015

The failing test seems to be unrelated to this change.

@geek

This comment has been minimized.

Copy link
Member

geek commented Aug 3, 2015

@gergoerdosi thanks, I am restarting the tests and checking on that error.

@gergoerdosi

This comment has been minimized.

Copy link
Contributor Author

gergoerdosi commented Aug 3, 2015

@geek Thanks! For some reason it fails locally on the master branch too. Maybe one of the dependencies got an update which broke something?

@geek

This comment has been minimized.

Copy link
Member

geek commented Aug 3, 2015

@gergoerdosi It looks like we have a bad assertion on https://github.com/hapijs/lab/blob/master/test/reporters.js#L831

Can you remove the , 'Working right' part... I know it's not really related to your PR, but it should resolve the breaking build.

It looks like the code module fixed a bug that caused this issue to surface.

@geek geek added the dependency label Aug 3, 2015
@geek geek added this to the 5.14.1 milestone Aug 3, 2015
@geek geek self-assigned this Aug 3, 2015
Gergo Erdosi
@gergoerdosi

This comment has been minimized.

Copy link
Contributor Author

gergoerdosi commented Aug 3, 2015

@geek Pushed the change, build is passing now.

@cjihrig

This comment has been minimized.

Copy link
Contributor

cjihrig commented Aug 3, 2015

With regards to code - we added an options argument to equal() for doing deep equal comparisons. The assertion in lab broke because it was passing an extra argument that was never supported.

geek added a commit that referenced this pull request Aug 3, 2015
Update eslint to 1.0.x
@geek geek merged commit e73d6ad into hapijs:master Aug 3, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gergoerdosi gergoerdosi deleted the gergoerdosi:eslint branch Aug 3, 2015
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented Aug 10, 2015

none of the new rules apply like return-callback? http://eslint.org/blog/2015/07/eslint-1.0.0-released/#new-rules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.