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

Build: Update devDependencies & jsHint configuration #259

Merged
merged 1 commit into from Apr 28, 2014
Merged

Conversation

mgol
Copy link
Member

@mgol mgol commented Apr 28, 2014

It's a good practice to list peerDependencies directly in package.json,
especially that current auto-installing behavior will go away in future
versions of npm: npm/npm#5080

Also, jsHint plugin should search for proper .jshintrc files on its own;
current behavior integrated badly with IDEs in some cases.

cc @markelog

It's a good practice to list peerDependencies directly in package.json,
especially that current auto-installing behavior will go away in future
versions of npm: npm/npm#5080

Also, jsHint plugin should search for proper .jshintrc files on its own;
current behavior integrated badly with IDEs in some cases.
@markelog
Copy link
Member

And also we would update deps, great :-).

current auto-installing behavior will go away in future

nice, hate peerDeps

current behavior integrated badly with IDEs in some cases

A bit worrisome, but i don't use them, so it's okay with me. New .jshintrc is a copy of https://github.com/jquery/sizzle/blob/master/.jshintrc, i wonder if there is a good way to no duplicate it

@mgol
Copy link
Member Author

mgol commented Apr 28, 2014

current behavior integrated badly with IDEs in some cases

A bit worrisome, but i don't use them, so it's okay with me.

This is because IDEs/editors usually defer to the default jsHint behavior, i.e. looking for .jshintrc up the directory tree and obviously ignoring Grunt configuration.

New .jshintrc is a copy of https://github.com/jquery/sizzle/blob/master/.jshintrc, i wonder if there is a good way to no duplicate it

We already duplicate a lot. Current jsHint allows to remove the duplication but not all editors have caught up yet; see jquery/jquery#1471; there was also discussion on our mailing list, @scottgonzalez was against using the extends option for now.

@markelog
Copy link
Member

Edit: sorry, wrong thread

@mgol
Copy link
Member Author

mgol commented Apr 28, 2014

This is the WebStorm bug that makes it impossible to use it with jsHint extends option:
http://youtrack.jetbrains.com/issue/WEB-10433
Other editors usually don't reimplement jsHint lookup algorithms which makes this issue not exist there.

@markelog
Copy link
Member

Current jsHint allows to remove the duplication but not all editors have caught up yet; see jquery/jquery#1471; there was also discussion on our mailing list, @scottgonzalez was against using the extends option for now

Yep, i remember, that is why i said "a good way"

"karma-browserstack-launcher": "0.0.8",
"karma-chrome-launcher": "0.1.3",
"karma-firefox-launcher": "0.1.3",
Copy link
Member Author

Choose a reason for hiding this comment

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

These two are technically not needed but with them one can test locally. I didn't add Safari or IE because they're not available on all 3 platforms.

I also didn't add the Opera launcher as it's still for the old Presto version, it's extremely slow and we're in the maintenance mode for Presto Opera anyway.

@timmywil
Copy link
Member

👍

@mgol mgol merged commit e0708a9 into jquery:master Apr 28, 2014
@mgol mgol deleted the build branch April 28, 2014 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants