search

manuelluis / jsrrdgraph

javascript rrdgraph
61 stars 12 forks source link

Fixed jshint warnings/errors #12

Closed Lekensteyn closed 9 years ago

Lekensteyn commented 9 years ago

Many fall in the category missing semicolon, but there are legitimate bugs (like throwing an error with an undefined variable, using isInfinite instead of !isFinite or fabs instead of Math.abs).

At some places, I moved the variable declarations to avoid duplicate definition warnings.

Redundant breaks have been removed (after return / throw).

Global variables were implicitly defined in RrdDataFile (which caught my attention) and Base64, these have been made local.

Also fixed some whitespace errors. Yay, the consistency. Not all (style) issues are fixed.

manuelluis commented 9 years ago

Thank you.

I'm not a big fan of some type of checks in jslint...

Lekensteyn commented 9 years ago

*jshint, not jslint.

The warnings make sense. Consider tabs vs spaces for example, inconsistency in its use make it less readable (when using git diff, the indent will change due to the + prefix).

Trailing semicolons, while not necessary it is much better to consistent. Wrt. break after throw/return, that may be a personal preference, but I think it is just distracting noise.

About x == 1 vs x === 1, that sloppiness may lead to bugs. As it is time-consuming to check the callers (and fixup if needed), I did not fix the majority of these cases.

Overall, the key is consistency.