closure fixes + cosmetic + jshintrc

[dmp42]

Hi there.

First changes are pretty harmless:

• a number of entirely cosmetic fixes (extra spaces, consistent quoting style, missing semicolon)
• trivial fixes (comparison to undefined, and simplify test in for)
• upgrade log4js to latest version

Second (more important) change is about the use of conditional compilation. Unfortunately closure will remove the /cc/ as with any other comment - hence the minified version is broken on IE... Thus I replaced it with an eval (in order to workaround closure) - dirty, but there really is no good solution IIRC. This is: https://github.com/jsBoot/console-shim/commit/b5d51864e9c302b0cc350e7cd294be5cfc5f453b

Third, I upgraded closure to the latest version - which in turn required type hinting adjustements to avoid warnings (closure really is damn pedant...). This is: https://github.com/jsBoot/console-shim/commit/57f9d0eaf081cec39f66bcb7a3e5d54eeb37e553 and https://github.com/jsBoot/console-shim/commit/9f353a0bf17165df74ad1c69720928ffb0e613be

... this allowed for ES5_STRICT minification, which I added to the build file: https://github.com/jsBoot/console-shim/commit/472deeeedfef9ad20cfb09d24c7b8931a57d6ba9

Finally, I commited a .jshintrc file for good measure.

Feel free to cherry pick what you deem appropriate, or ask to amend this pull request if you think something is bad.

Cheers.

[buildhive]

Klaus Reimer » console-shim #11 SUCCESS This pull request looks good (what's this?)

[kayahr]

Thanks. I have pulled most of your commits, except of the following:

e0e333a: I like to keep the condition as it is because this one-liner is a pattern I always use in for...in loops. Makes it easier to check (by automatic searching) if this necessary check is missing in a loop. So this one-liner more belongs to the for-statement and not to the code inside the for block. That's why it makes more sense to keep it as it is.

38f4e8d: Another pattern I like to keep. I want to prevent checks against "undefined" when possible and I want to treat undefined and null as same value when possible. !=null is true when the variable is not null AND not undefined. That's exactly what I want.

53c3159: I guess this jshintrc file is for some JavaScript validation tool? Well there are dozens of such programs out there and I can't include the configs for all of them in the project. But I'll take a look at this tool. Maybe it convinces me to use it.

[dmp42]

Thank you very much for your time reviewing all this.

• the reasoning about for... in / hasOwnProperty check: this is typically a case where jshint is useful (warn automatically if that check is missing - this is the "forin: true" option) - saving the need to do manual search
• about the check vs. null / undefined: ok, your choice, I can live with that :)
• about jshint: AFAIK, there is two widely used tools like this - the "original" jslint, and jshint which is a (more configurable) fork. Documentation is here http://www.jshint.com/docs/ and you install/run it with node/npm IIRC. The jshintrc file is a way to "customize" jshint behavior, which you can also do inline using comments instead (eg: /jshint someoption:true, otheroption:false/). I certainly understand you don't want to clutter (this) project(s) with tons of configuration files. That being said, using a code quality tool like that (integrated into the build system, ideally) really has some value. Either way, this is just a suggestion obviously - and this is (also) what forks are for, and I'm quite happy either way ;)

Best regards, and have a merry XMas.