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

Add a complete jshintrc #662

Closed
wants to merge 2 commits into from
Closed

Add a complete jshintrc #662

wants to merge 2 commits into from

Conversation

baer
Copy link
Contributor

@baer baer commented Sep 3, 2014

This replaces the existing .jshintrc file with the complete one from https://github.com/jshint/jshint/blob/master/examples/.jshintrc - replacing values where needed. There are 3 intentions behind this PR.

  • Updating the .jshintrc when properties are deprecated/added is much easier when you are just tracking changes in the project's repo
  • This version gives visibility into what rules can be tightened or loosened as the project needs them.
  • This version explicitly enumerates the rules since declarative > imperative

I also noticed that the test folder does not pass jshint. It's mostly quotation marks and line length but that may be worth fixing eventually.

@phated
Copy link
Member

phated commented Sep 13, 2014

Even though jshint supports json with comments, I don't like this strategy for a .jshintrc because other libraries might parse it with JSON.parse

@jednano
Copy link
Contributor

jednano commented Sep 15, 2014

@phated that's why @sindresorhus wrote strip-json-comments

@baer
Copy link
Contributor Author

baer commented Sep 15, 2014

I agree, there are some easy tools to strip comments if you need to and I think that the comments make the file more useful since some of the options (and some of the warning messages) in jshint can be a bit cryptic. It gives you something to grep against.

@sindresorhus
Copy link
Contributor

Suppose you are using JSON to keep configuration files, which you would like to annotate. Go ahead and insert all the comments you like. Then pipe it through JSMin before handing it to your JSON parser. - Douglas Crockford, 2012

@baer
Copy link
Contributor Author

baer commented Sep 15, 2014

image

@yocontra
Copy link
Member

yocontra commented Oct 9, 2014

I don't think we need to fill in the defaults here, going to close this

@yocontra yocontra closed this Oct 9, 2014
@baer
Copy link
Contributor Author

baer commented Oct 9, 2014

The reason that you may want to fill in the the defaults is that unlike some cli programs, in jshint not defining an option does not disable it, it will just run the check using it's default value. This PR shows you what you may not realize you are even checking.

@yocontra
Copy link
Member

yocontra commented Oct 9, 2014

@baer I'm okay with the default values. Anything I want non-default is specified in the configuration file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants