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

Linting errors + 'bench' not working on Windows #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshq00
Copy link

@joshq00 joshq00 commented Jan 24, 2015

  • Added .jshintrc
  • Removed unneeded variables / matched quotes / == to ===
  • benchmark/parse_map.js
    • was trying to split by '\r\n' on Windows. Changed to /\r?\n/
    • instead of calling toString, read file by utf8 encoding

@joshq00
Copy link
Author

joshq00 commented Jan 30, 2015

Mine is unset. But, either way, the regular expression will work, while the EOL assumes the files have the same line endings your os uses

"eqeqeq": true,
"quotmark": true,
"smarttabs": true,
"trailing": true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do smarttabs and trailing options do? I couldn't find them in the jshint docs.

Copy link
Author

Choose a reason for hiding this comment

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

Smart tabs: http://jshint.com/docs/faq/

Trailing: looks like this was removed with v2.5, it warned for trailing
whitespace on lines

On Sunday, February 1, 2015, Raminder Singh [email protected]
wrote:

In .jshintrc
#88 (comment):

@@ -0,0 +1,17 @@
+{

  • "browser": false,
  • "node": true,
  • "globals": {
  • "require": true
  • },
  • "predef": {
  • },
  • "eqeqeq": true,
  • "quotmark": true,
  • "smarttabs": true,
  • "trailing": true,

What do smarttabs and trailing options do? I couldn't find them in the
jshint docs.

Reply to this email directly or view it on GitHub
https://github.com/qiao/PathFinding.js/pull/88/files#r23895774.

@imor
Copy link
Collaborator

imor commented Feb 2, 2015

@joshq00 Everything looks fine to me. Thanks for the patch 👍
@qiao Any questions you have or should I go ahead and merge?

@joshq00
Copy link
Author

joshq00 commented Oct 8, 2016

?

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.

3 participants