I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be too inconvenient for daily usage, or even cause some problems (i.e. with pootle, jenkins).
I think running jshint one way or another is quite useful. Just going over existing code to fix the warnings revealed a bug[1], which would have been noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't
allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be
too inconvenient for daily usage, or even cause some problems (i.e. with
pootle, jenkins).
if code you want to commit doesn't even pass the jshint validation baseline... then you better don't commit it.
Not running jshint on a per-commit basis makes errors creep in and if you use it and want to commit something, you might have to fix errors produced by someone else.
I think running jshint one way or another is quite useful. Just going
over existing code to fix the warnings revealed a bug[1], which would
have been noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
restoring what has been there more than a year ago (roughly)?
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't allow
you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step,
We do!
as it may be too inconvenient for daily usage, or even cause some
problems (i.e. with pootle, jenkins).
Thanks for commenting Ingo!
I think running jshint one way or another is quite useful. Just going over
existing code to fix the warnings revealed a bug[1], which would have been
noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
I say: if no developer objects by tonight, let's just go ahead with it.
Emil
···
On Friday, September 11, 2015, Boris Grozev <boris@jitsi.org> wrote:
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't
allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be
too inconvenient for daily usage, or even cause some problems (i.e. with
pootle, jenkins).
if code you want to commit doesn't even pass the jshint validation
baseline... then you better don't commit it.
My point is that you shouldn't *push* code that doesn't pass jshint. But I often commit stuff locally that is never meant to be pushed, and this makes it inconvenient.
Not running jshint on a per-commit basis makes errors creep in and if
you use it and want to commit something, you might have to fix errors
produced by someone else.
I think running jshint one way or another is quite useful. Just going
over existing code to fix the warnings revealed a bug[1], which would
have been noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
restoring what has been there more than a year ago (roughly)?
Fixing warnings in code which did not exist a year ago.
What do you think about also running jshint as part of the app target in the Makefile?
Devin
···
From: dev [mailto:dev-bounces@jitsi.org] On Behalf Of Emil Ivov
Sent: Friday, September 11, 2015 8:35 AM
To: Jitsi Developers
Subject: Re: [jitsi-dev] Using jshint in jitsi-meet
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step,
We do!
as it may be too inconvenient for daily usage, or even cause some problems (i.e. with pootle, jenkins).
Thanks for commenting Ingo!
I think running jshint one way or another is quite useful. Just going over existing code to fix the warnings revealed a bug[1], which would have been noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
I say: if no developer objects by tonight, let's just go ahead with it.
--
sent from my mobile
This email message is for the sole use of the intended recipient(s) and may contain information that is privileged, confidential, and exempt from disclosure under applicable law. Any unauthorized review, use, copying, disclosure or dissemination is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't
allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be
too inconvenient for daily usage, or even cause some problems (i.e. with
pootle, jenkins).
Pootle already has uses some hooks. I don't know anything about npm, but as Pootle never executes anything from the checked out repository I think it should be safe to use.
I think running jshint one way or another is quite useful. Just going
over existing code to fix the warnings revealed a bug[1], which would
have been noticed and fixed right away if we'd used jshint at the time.
Any thoughts about merging either part 1 or both into master?
I'm generally very much in favor of using as many of such tools as possible. Technically enforcing people to adhere to a certain set of rules is the best way to keep (accidental) sloppiness away. Even more so if it has the potential of catching bugs.
travis is very useful for enforcing jshint on merges, but that requires a development process where people make PRs instead of committing to master.
···
Am 11.09.2015 um 08:02 schrieb Boris Grozev:
On 11/09/15 09:18, Philipp Hancke wrote:
Am 11.09.2015 um 07:07 schrieb Boris Grozev:
Hey,
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't
allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be
too inconvenient for daily usage, or even cause some problems (i.e. with
pootle, jenkins).
if code you want to commit doesn't even pass the jshint validation
baseline... then you better don't commit it.
My point is that you shouldn't *push* code that doesn't pass jshint. But
I often commit stuff locally that is never meant to be pushed, and this
makes it inconvenient.
That's exactly what I need, so my concern is invalid. Thanks!
Boris
···
On 11/09/15 10:12, Philipp Hancke wrote:
Am 11.09.2015 um 08:02 schrieb Boris Grozev:
On 11/09/15 09:18, Philipp Hancke wrote:
Am 11.09.2015 um 07:07 schrieb Boris Grozev:
Hey,
I've created a 'lint' branch[0] in Jitsi-Meet that:
1. Fixes (or disables) all jshint warnings.
2. Includes "precommit-hook" from npm, which runs jshint and doesn't
allow you to commit unless it passes cleanly.
I'm not sure that we want to go as far as that second step, as it may be
too inconvenient for daily usage, or even cause some problems (i.e. with
pootle, jenkins).
if code you want to commit doesn't even pass the jshint validation
baseline... then you better don't commit it.
My point is that you shouldn't *push* code that doesn't pass jshint. But
I often commit stuff locally that is never meant to be pushed, and this
makes it inconvenient.