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

Added space after comment rule #381

Merged
merged 1 commit into from Jul 1, 2015

Conversation

@AdriVanHoudt
Copy link
Contributor

AdriVanHoudt commented Jun 23, 2015

Another eslint, another PR (until v1 :P)

  • updated eslint to 0.23
  • added space after comment rule, except when followed by , to allow comments for optional function params like function (title /*, options, fn */)
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jun 23, 2015

hmm tests fail because

lib/reporters/console.js:
        Line 43: space-unary-ops - Unary word operator "this" must be followed by whitespace.

And actually doing this .count++; does not fail any tests, weird

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jun 24, 2015

I talked to the people of eslint (see linked issue) and a fix will probably land this weekend so I would hold off on this PR until then, either we jump 2 releases or an extra patch, both are nice

@geek geek removed this from the 5.11.1 milestone Jun 25, 2015
@geek geek modified the milestone: 5.11.2 Jun 30, 2015
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jul 1, 2015

@geek how do you want me to proceed? update the pr with #387 and just add the new rule, make a new pr with the rule or just close if you don't want the rule

@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 1, 2015

I would update the PR with the new rule... delete the change to package.json and it should merge. Right?

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jul 1, 2015

My rebase skills are not top notch :P but it works, will try to squash

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jul 1, 2015

ok that's better ^^ force push 🎉

@geek

This comment has been minimized.

Copy link
Member

geek commented Jul 1, 2015

@AdriVanHoudt can't beat a good force push

@geek geek added this to the 5.11.2 milestone Jul 1, 2015
geek added a commit that referenced this pull request Jul 1, 2015
updated eslint to 0.23.x from 0.22.x
@geek geek merged commit c86ae5c into hapijs:master Jul 1, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@geek geek changed the title updated eslint to 0.23.x from 0.22.x Added space after comment rule Jul 1, 2015
@geek geek added feature and removed dependency labels Jul 1, 2015
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor Author

AdriVanHoudt commented Jul 1, 2015

@geek the one advantage in the war with git 💪 😄

@AdriVanHoudt AdriVanHoudt deleted the AdriVanHoudt:eslint23 branch Jul 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.