jish / pre-commit

A slightly improved pre-commit hook for git
https://jish.github.io/pre-commit/
Other
796 stars 96 forks source link

Update the version of JSHint bundled with pre-commit #157

Closed jish closed 10 years ago

jish commented 10 years ago

Currently is it not possible to access a JavaScript object with a property named public. If you use object.public JSHint complains, and likewise, it complains using the alternate syntax object['public'].

If you are inspecting a third party JSON API and do not have control over this convention, you are forbidden from committing any code using a property named public.

Newer versions of JSHint allow object.public notation.

madhums commented 10 years ago

+1

jish commented 10 years ago

Currently, we have a vendored copy of JSHint checked into the repo, we might want to investigate using a gem or something that gets updated more frequently. (or not, we can just manage our own version independently).

The only two JSHint gems I can find are:

I'm not sure how frequently or supported either of them are... =/

mpapis commented 10 years ago

@jish the most secure thing would be to push the jshint managed by our own, maybe even a clone of one of the existing gems, but I think it has to be in our control as the mentioned gems are out of date. btw how is it going with picking a common repo for pre-commit related gems?

madhums commented 10 years ago

is it anyway possible to use the node version of jsHint ? I understand pre-commit is in ruby, but if there is any way to execute and get the results back

mpapis commented 10 years ago

right now we use ExecJS, it will use node when it is available, but it will be also be able to use number of other available js engines, so limiting to node would be a step back in my opinion.

mpapis commented 10 years ago

@jish I guess we need a decision here, which route do we go ( I might be able to spare some time for simpler solutions)

jish commented 10 years ago

I sat down and tried an upgrade the other day, but I think we patched the vendored JavaScript code in our copy of JSHint. So we can't just do an drop in upgrade :(

I might need @shajith to chime in if he remembers how we patched it.