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

Switch to JSCS #12853

Merged
merged 8 commits into from
Mar 5, 2014
Merged

Switch to JSCS #12853

merged 8 commits into from
Mar 5, 2014

Conversation

XhmikosR
Copy link
Member

Since JSHint plans to remove the style changes come v3, or move them to plugins, and we already use JSCS, I guess it makes sense to use that for the current checks.

Thanks to @mikesherov for working on fixing the issues we had upstream!

/CC @cvrebert @mdo

TODO:

  • Move camelcase to JSCS requireCamelCaseOrUpperCaseIdentifiers
  • Move trailing to JSCS disallowTrailingWhitespace
  • Move laxbreak to JSCS "requireOperatorBeforeLineBreak": ["?", "+", "-", "/", "*", "=", "==", "===", "!=", "!==", ">", ">=", "<", "<="],
  • Move immed to JSCS requireParenthesesAroundIIFE

"node" : true,
"nonbsp" : true,
"strict" : true,
"trailing" : true,

Choose a reason for hiding this comment

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

going away in JSHint 3: jscs-dev/node-jscs#102

@XhmikosR
Copy link
Member Author

@mikesherov: thanks for chiming in. I will replace those with the JSCS counterparts tomorrow.

@cvrebert cvrebert added the grunt label Feb 27, 2014
@XhmikosR XhmikosR added this to the v3.2.0 milestone Feb 27, 2014
@XhmikosR XhmikosR self-assigned this Feb 27, 2014
@XhmikosR
Copy link
Member Author

Currently, tests fail due to the requireOperatorBeforeLineBreak check. So after we fix this, we can merge it.

I also went ahead and used the same target files as JSHint for JSCS to reduce duplication.

@mikesherov
Copy link

Right. If you don't care about where the operators go, use no rule (which is the equivalent of laxbreak). Glad JSCS was able to provide a bit of extra checking for you :)

@mikesherov
Copy link

Btw, indentation checks for chained function calls is something we intend on adding the next JSCS. Would you be interested in this check? Just want to gauge priority.

@XhmikosR
Copy link
Member Author

@mikesherov: thanks for all your feedback!

I just kept disallowLeftStickedOperators for the time being to match the current behavior. We still need to keep laxbreak for JSHint though.

Improving the checks for chained function calls would be something we could use, but I don't think it's important. I mean, there are so many more important issues on the JSCS issue tracker :)

@mdo @cvrebert: Feel free to comment on the PR so that we merge it.

@cvrebert
Copy link
Collaborator

LGTM.

XhmikosR added a commit that referenced this pull request Mar 5, 2014
@XhmikosR XhmikosR merged commit d7dc8a7 into master Mar 5, 2014
@XhmikosR XhmikosR deleted the jscs branch March 5, 2014 06:42
@mdo mdo mentioned this pull request Mar 7, 2014
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants