Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Build: Add config files for jscs and jshint, run jshint with npm-test #37

Merged
merged 1 commit into from Jul 30, 2015

Conversation

jzaefferer
Copy link
Member

Fixes 3 'lint' issues: Remove unused var, replace semicolon with comma, define Promise (even though Promise is 'now' native in nodejs).

There's currently lots of whitespace issues, those need to be addressed separately, then jscs can be regularly checked as well.

@@ -17,7 +17,7 @@
],
"scripts": {
"start": "./bin/server.js",
"test": "nodeunit"
"test": "jshint bin/ lib/ test/ *.js && nodeunit"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldnt we add jscs here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I mentioned this in the commit message (copied into the PR description):

There's currently lots of whitespace issues, those need to be addressed separately, then jscs can be regularly checked as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Though I could add "requireSpacesInsideParentheses": null to the config to turn of that one rule and fix the rest.

Copy link
Member

Choose a reason for hiding this comment

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

Should the exceptions/ directory also be linted? I know it's just some arrays but there are still simple mistakes we can make there.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add that, there's just two issues that I can fix quickly enough.

Copy link
Member

Choose a reason for hiding this comment

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

if its just white space just run jscs with fix and do it as part of this its a small enough repo.

@jzaefferer
Copy link
Member Author

Updated, now running both jshint and jscs as part of npm-test.

Fixes spacing and some other lint issues.

Closes jquery#37
@jzaefferer
Copy link
Member Author

Updated again, now jshint and jscs validate all js files in the repo (both tools ignore node_modules, it seems), whitespace issues are fixed as well.

@arschmitz
Copy link
Member

Looks good to me. 👍

@jzaefferer jzaefferer merged commit 2e27baf into jquery:master Jul 30, 2015
@jzaefferer jzaefferer deleted the linting branch July 30, 2015 14:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants