Skip to content

Switch to JSCS#12853

Merged
XhmikosR merged 8 commits intomasterfrom
jscs
Mar 5, 2014
Merged

Switch to JSCS#12853
XhmikosR merged 8 commits intomasterfrom
jscs

Conversation

@XhmikosR
Copy link
Copy Markdown
Member

@XhmikosR XhmikosR commented Feb 26, 2014

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

mikesherov
mikesherov reviewed Feb 26, 2014
View reviewed changes
grunt/.jshintrc Outdated
Copy link
Copy Markdown

@mikesherov mikesherov Feb 26, 2014

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
Copy Markdown
Member Author

XhmikosR commented Feb 26, 2014

@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
Copy Markdown
Member Author

XhmikosR commented Feb 28, 2014

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
Copy Markdown

mikesherov commented Feb 28, 2014

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
Copy Markdown

mikesherov commented Feb 28, 2014

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
Copy Markdown
Member Author

XhmikosR commented Feb 28, 2014

@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
Copy Markdown
Collaborator

cvrebert commented Feb 28, 2014

LGTM.

@XhmikosR XhmikosR mentioned this pull request Mar 3, 2014
Merged
4 tasks
XhmikosR added a commit that referenced this pull request Mar 5, 2014
@XhmikosR
Merge pull request #12853 from twbs/jscs
d7dc8a7 
Switch to JSCS
@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
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@XhmikosR XhmikosR

Labels

build

Projects

None yet

Milestone

v3.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@XhmikosR @mikesherov @cvrebert