-
Notifications
You must be signed in to change notification settings - Fork 195
JSHint: enforce more rules #323
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
Conversation
4baab2e to
0f062d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Indigo744 ,
Thanks for your PR. I think it will lead to a great improvement of the overall quality of the code.
I have just noticed what seems to be a little mistake in the CONTRIBUTING.md (I hope your adds to this file will motivate some more users to contribute !).
When fixed, everything should be ok to be merged !
CONTRIBUTING.md
Outdated
| ## Test suite | ||
|
|
||
| There is a test suite in the `/test` folder, power by [QUnit](http://qunitjs.com/). | ||
| While not broken, it is however, it is very light. We sadly don't have the time to expand it for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a small mistake here ?
it is however, it is very light. => it is however very light.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
|
I've squashed and merged this PR (using option provided by GH), so there's not too much commit! I think we should try to do this as much as possible to reduce cluttering the commit tree :) |
|
Thanks for the fix and for the merge ! You juste made me discover this shortcut on github, I didn't know about it before ... I agree with you, it will allow the project history to be more clear and simple, I will try to think about it for the future merges ! |
I suggest we enforce more JSHint rules to bring Mapael up to current JS standard.
Also, it helps us catch more bug or typo (see the one at line 1470 that was not detected before).
The following JSHint rules are added:
The following options are also added, only to remove some wrong warnings:
The following JSHint rules are deleted (no longer relevant):
Also, all comments are removed from .jshintrc file as they are forbidden by JSON standard.
Instead, justification is in a Wiki page: https://github.com/neveldo/jQuery-Mapael/wiki/JSHint