Skip to content

Rename jshintrc to .jshintrc#421

Merged
ljharb merged 1 commit into
airbnb:masterfrom
banhathome:jshintrc-fix
Dec 23, 2015
Merged

Rename jshintrc to .jshintrc#421
ljharb merged 1 commit into
airbnb:masterfrom
banhathome:jshintrc-fix

Conversation

@banhathome
Copy link
Copy Markdown
Contributor

According to http://jshint.com/docs/, the "special file" to configure JSHint is named .jshintrc, not jshintrc.

@banhathome
Copy link
Copy Markdown
Contributor Author

#134 is where the jshintrc file was added. The description of the PR says that "This adds a .jshintrc to the linters/ directory", when that wasn't actually the case. This pull request corrects this.

@justjake
Copy link
Copy Markdown
Collaborator

I'm split. On one hand, that is the proper name of the file for a project. On the other, we aren't actually linting any code in our linters directory, and users are expected to copy the file into their project anyways. Dotfiles are harder to work with: by default my editor hides them in the sidebar, and I can't tab complete them on the terminal. I would prefer to keep this as jshintrc and rely on users copying it the correct filename in their projects.

@banhathome
Copy link
Copy Markdown
Contributor Author

@justjake I didn't realize that this was intentional. Seeing as how the other file in the directory is a dotfile, and the pull request I referenced says that it adds a .jshintrc file, I assumed that this was a mistake.

@justjake
Copy link
Copy Markdown
Collaborator

it was probably a mistake back then -- i was expressing my personal opinion

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Dec 23, 2015

I'd love to hear some more opinions on this - personally, I don't find dotfiles difficult to work with, and I already have to configure my editor to show dotfiles to work with .eslintrc, .gitignore, and many others. I'm a weak +1 in favor of this.

@banhathome banhathome changed the title Renamed jshintrc to .jshintrc Rename jshintrc to .jshintrc Dec 23, 2015
@goatslacker
Copy link
Copy Markdown
Collaborator

+1 it's better to copy over the file without having to rename it.

Plus someone could forget to rename it and then it won't work for them.

@banhathome
Copy link
Copy Markdown
Contributor Author

I think it should be changed to a dotfile because:

  1. It needs to be a dotfile to work.
  2. The other file in the directory is a dotfile. If we decide to not have it as a dotfile, then .eslintrc should be renamed.

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Dec 23, 2015

@chrisngobanh that's a strong argument too. (would you mind freshly rebasing this on latest master, btw?)

According to http://jshint.com/docs/, the "special file" to configure JSHint is named `.jshintrc`, not `jshintrc`.
@banhathome
Copy link
Copy Markdown
Contributor Author

@ljharb Done!

Side question: Is the jshintrc file still being maintained? It looks out of date

@ljharb
Copy link
Copy Markdown
Collaborator

ljharb commented Dec 23, 2015

Somehow I doubt it. We may end up removing the file anyways - jshint's ES6 support is limited.

ljharb added a commit that referenced this pull request Dec 23, 2015
@ljharb ljharb merged commit 8815205 into airbnb:master Dec 23, 2015
@banhathome banhathome deleted the jshintrc-fix branch December 23, 2015 20:56
@ljharb ljharb mentioned this pull request Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants