info
Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
Using jshint as a commit hook
139 views
Skip to first unread message
Julien Wajsberg
Jan 20, 2014, 9:23:58 AM1/20/14
to dev-...@lists.mozilla.org
Hello people of Gaia,
Bug 937431 [1] is waiting a review+ to land, so now is a good time to
explain you the (hopefully good) changes this bug will give.
The README at [2] gives all information but here is an abstract.
Goals
====
The first goal of this work is to absolutely not disturb you from what
you're currently doing. That means that for existing files that don't
pass jshint, nothing changes.
The second goal of this work is to slowly move to jshint to lint our files.
Gjslint or jshint ?
==========
So we took the list of the existing files that don't pass jshint, and
we've put it in a file. When you commit some file, or when you run "make
lint", or when travis is running the "linters" job, we look into this
list and if your file is part of this list, we use gjslint to lint it.
Otherwise, we use jshint.
This also means that all new files will by default use jshint as linter.
Obviously, the goal is to make this list shrink until it's empty. So
here's the deal: if you remove a line from this list (which means this
file either was deleted or is passing jshint), you can commit. But if
you add a line to this list (which means a new file does not pass
jshint), then I'd like that you take 10 minutes to fix whatever is wrong
in this file.
jshint configuration
============
Intentionally, the main .jshintrc (the jshint configuration file) has
stricter rules than what you're used to with gjslint. Especially:
* you need 'use strict'; in all files
* it triggers an error when you're using a variable that is not defined
in this scope (eg: globals). You can define globally (ah ah) for your
app or subdirectories the globals that you can use.
* it requires braces even for one-line loops and if-blocks
See the README file [2] to fix these.
Overriding rules for some directories
====================
There is a Really Good Thing in the current version of jshint: you can
put a .jshintrc file in a directory and it will be used instead of the
main configuration file when jshint will lint files in this directory.
And you can also extend (inherit) from another .jshintrc file. So that
makes it very easy to use the main .jshintrc file and relax or stricten
some of the rules, or add predefined globals.
I already added those overriding rules for unit and marionette tests, in
all existing directories.
If you have other questions or suggestions, please ask! This is your
tool, use it to improve the quality of Gaia!
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=937431
[2]
https://github.com/julienw/gaia/blob/937431-jshint-hook/build/jshint/README.md
(will be in
https://github.com/mozilla-b2g/gaia/blob/master/build/jshint/README.md
after landing)
--
Julien
Bug 937431 [1] is waiting a review+ to land, so now is a good time to
explain you the (hopefully good) changes this bug will give.
The README at [2] gives all information but here is an abstract.
Goals
====
The first goal of this work is to absolutely not disturb you from what
you're currently doing. That means that for existing files that don't
pass jshint, nothing changes.
The second goal of this work is to slowly move to jshint to lint our files.
Gjslint or jshint ?
==========
So we took the list of the existing files that don't pass jshint, and
we've put it in a file. When you commit some file, or when you run "make
lint", or when travis is running the "linters" job, we look into this
list and if your file is part of this list, we use gjslint to lint it.
Otherwise, we use jshint.
This also means that all new files will by default use jshint as linter.
Obviously, the goal is to make this list shrink until it's empty. So
here's the deal: if you remove a line from this list (which means this
file either was deleted or is passing jshint), you can commit. But if
you add a line to this list (which means a new file does not pass
jshint), then I'd like that you take 10 minutes to fix whatever is wrong
in this file.
jshint configuration
============
Intentionally, the main .jshintrc (the jshint configuration file) has
stricter rules than what you're used to with gjslint. Especially:
* you need 'use strict'; in all files
* it triggers an error when you're using a variable that is not defined
in this scope (eg: globals). You can define globally (ah ah) for your
app or subdirectories the globals that you can use.
* it requires braces even for one-line loops and if-blocks
See the README file [2] to fix these.
Overriding rules for some directories
====================
There is a Really Good Thing in the current version of jshint: you can
put a .jshintrc file in a directory and it will be used instead of the
main configuration file when jshint will lint files in this directory.
And you can also extend (inherit) from another .jshintrc file. So that
makes it very easy to use the main .jshintrc file and relax or stricten
some of the rules, or add predefined globals.
I already added those overriding rules for unit and marionette tests, in
all existing directories.
If you have other questions or suggestions, please ask! This is your
tool, use it to improve the quality of Gaia!
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=937431
[2]
https://github.com/julienw/gaia/blob/937431-jshint-hook/build/jshint/README.md
(will be in
https://github.com/mozilla-b2g/gaia/blob/master/build/jshint/README.md
after landing)
--
Julien
Julien Wajsberg
Jan 20, 2014, 9:27:22 AM1/20/14
to dev-...@lists.mozilla.org
and I forgot to thank Corey from Bocoup who wrote most of this patch :)
> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
Julien Wajsberg
Jan 22, 2014, 4:24:47 AM1/22/14
to dev-...@lists.mozilla.org
Just landed !
What do you need to do ?
* run `make .git/hooks/pre-commit` to install the new commit hook (note:
this is done automatically when you run most make goals)
* run `npm install` to install the latest jshint version (note: this is
done automatically when you run `make hint` or `make lint`
* start to fix jshint issues.
If you find any issues while I'm here, please ping me on IRC and I'll
have a look. If this prevents you from committing for something that
looks wrong, we can still back out.
Have a nice day !
What do you need to do ?
* run `make .git/hooks/pre-commit` to install the new commit hook (note:
this is done automatically when you run most make goals)
* run `npm install` to install the latest jshint version (note: this is
done automatically when you run `make hint` or `make lint`
* start to fix jshint issues.
If you find any issues while I'm here, please ping me on IRC and I'll
have a look. If this prevents you from committing for something that
looks wrong, we can still back out.
Have a nice day !
James Burke
Jan 22, 2014, 4:49:37 AM1/22/14
to Julien Wajsberg, dev-...@lists.mozilla.org
One thing that has bit me in the past when I had an out of date
gjslint, was that doing a master merge onto my branch then committing
the merge result would run the linting on files I did not change
myself, but were just merged files.
I would upgrade gsjlint then it was fine, but I wonder if something
like that could happen with this change, where I would not necessarily
be in control of a direct fix:
The files merged in were not edited after this change was done, so
they could have jshint issues. Because of that, the merge would fail
since the lint will run on those files, and the branch would be stuck
in a bad state.
I am happy to see the change to jshint, so for myself, I am willing to
just redo my branches by doing fresh branches from latest master and
applying a patch on top of that. Just calling it out as a possible
issue that could generate feedback.
James
gjslint, was that doing a master merge onto my branch then committing
the merge result would run the linting on files I did not change
myself, but were just merged files.
I would upgrade gsjlint then it was fine, but I wonder if something
like that could happen with this change, where I would not necessarily
be in control of a direct fix:
The files merged in were not edited after this change was done, so
they could have jshint issues. Because of that, the merge would fail
since the lint will run on those files, and the branch would be stuck
in a bad state.
I am happy to see the change to jshint, so for myself, I am willing to
just redo my branches by doing fresh branches from latest master and
applying a patch on top of that. Just calling it out as a possible
issue that could generate feedback.
James
Julien Wajsberg
Jan 22, 2014, 4:57:37 AM1/22/14
to James Burke, dev-...@lists.mozilla.org
Le 22/01/2014 10:49, James Burke a écrit :
> One thing that has bit me in the past when I had an out of date
> gjslint, was that doing a master merge onto my branch then committing
> the merge result would run the linting on files I did not change
> myself, but were just merged files.
>
> I would upgrade gsjlint then it was fine, but I wonder if something
> like that could happen with this change, where I would not necessarily
> be in control of a direct fix:
>
> The files merged in were not edited after this change was done, so
> they could have jshint issues. Because of that, the merge would fail
> since the lint will run on those files, and the branch would be stuck
> in a bad state.
>
> I am happy to see the change to jshint, so for myself, I am willing to
> just redo my branches by doing fresh branches from latest master and
> applying a patch on top of that. Just calling it out as a possible
> issue that could generate feedback.
>
I'm not sure I followed everything but:
> One thing that has bit me in the past when I had an out of date
> gjslint, was that doing a master merge onto my branch then committing
> the merge result would run the linting on files I did not change
> myself, but were just merged files.
>
> I would upgrade gsjlint then it was fine, but I wonder if something
> like that could happen with this change, where I would not necessarily
> be in control of a direct fix:
>
> The files merged in were not edited after this change was done, so
> they could have jshint issues. Because of that, the merge would fail
> since the lint will run on those files, and the branch would be stuck
> in a bad state.
>
> I am happy to see the change to jshint, so for myself, I am willing to
> just redo my branches by doing fresh branches from latest master and
> applying a patch on top of that. Just calling it out as a possible
> issue that could generate feedback.
>
* first, it's a lot easier to upgrade jshint, and it's done
automatically when you run make hint. (not when you commit though, I
didn't want to initiate a network connection in that case)
* then, there is a file (called xfail.list) which lists all files that
currently fail jshint. I think it would be merged in with the failed
files, right ? And then their errors would be ignored.
Does this answer your question?
--
Julien
Corey Frang
Jan 22, 2014, 8:39:08 AM1/22/14
to
I'd like to say, I think xfail.list should be a "remove only" kind of list :)
Greg Weng
Jan 22, 2014, 10:06:11 AM1/22/14
to Corey Frang, dev-gaia
I've found the unit tests has different rules...
'use strict' in app/js can pass if it got placed as the first line, but in
test it would be failed and must be put in a anonymous function. Other
strange errors like there is that the JSHint would complain no 'window' or
'self', but it would not complain this for other files.
The strangest thing is that if I copy the content passed the JSHint in the
normal files (not test), to a test file in the unit test directory, it
would complain, even for the same content!
This is bothers me because when I pushed it to the Travis, these different
checking rules make my patch got blocked. And I don't want to give up to
set an entry in the xfalil.list ...
2014/1/22 Corey Frang <gna...@gmail.com>
<http://about.me/snowmantw>Greg Weng
http://about.me/snowmantw
*Understand y f = f [ y f ] ; lose last remaining non-major friend*
* -- Anonymous*
'use strict' in app/js can pass if it got placed as the first line, but in
test it would be failed and must be put in a anonymous function. Other
strange errors like there is that the JSHint would complain no 'window' or
'self', but it would not complain this for other files.
The strangest thing is that if I copy the content passed the JSHint in the
normal files (not test), to a test file in the unit test directory, it
would complain, even for the same content!
This is bothers me because when I pushed it to the Travis, these different
checking rules make my patch got blocked. And I don't want to give up to
set an entry in the xfalil.list ...
2014/1/22 Corey Frang <gna...@gmail.com>
> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
>
--
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
>
<http://about.me/snowmantw>Greg Weng
http://about.me/snowmantw
*Understand y f = f [ y f ] ; lose last remaining non-major friend*
* -- Anonymous*
Greg Weng
Jan 22, 2014, 10:10:35 AM1/22/14
to Corey Frang, dev-gaia
(Sorry so may typos because I was writing in a hurry. Summarize it as
follow):
1. Copy the content passed JSHint from a normal JS file inside an app to a
unit test file would fail the linter.
2. Travis would check the test files as normal files, but because of the
different rules, they would be failed
3. To add test files in xfall.list seems a easier workaround, but I don't
think would solve the problem.
2014/1/22 Greg Weng <snow...@gmail.com>
> I've found the unit tests has different rules...
>
> 'use strict' in app/js can pass if it got placed as the first line, but in
> test it would be failed and must be put in a anonymous function. Other
> strange errors like there is that the JSHint would complain no 'window' or
> 'self', but it would not complain this for other files.
>
> The strangest thing is that if I copy the content passed the JSHint in the
> normal files (not test), to a test file in the unit test directory, it
> would complain, even for the same content!
>
> This is bothers me because when I pushed it to the Travis, these different
> checking rules make my patch got blocked. And I don't want to give up to
> set an entry in the xfalil.list ...
>
>
> 2014/1/22 Corey Frang <gna...@gmail.com>
>
follow):
1. Copy the content passed JSHint from a normal JS file inside an app to a
unit test file would fail the linter.
2. Travis would check the test files as normal files, but because of the
different rules, they would be failed
3. To add test files in xfall.list seems a easier workaround, but I don't
think would solve the problem.
2014/1/22 Greg Weng <snow...@gmail.com>
> I've found the unit tests has different rules...
>
> 'use strict' in app/js can pass if it got placed as the first line, but in
> test it would be failed and must be put in a anonymous function. Other
> strange errors like there is that the JSHint would complain no 'window' or
> 'self', but it would not complain this for other files.
>
> The strangest thing is that if I copy the content passed the JSHint in the
> normal files (not test), to a test file in the unit test directory, it
> would complain, even for the same content!
>
> This is bothers me because when I pushed it to the Travis, these different
> checking rules make my patch got blocked. And I don't want to give up to
> set an entry in the xfalil.list ...
>
>
> 2014/1/22 Corey Frang <gna...@gmail.com>
>
Corey Frang
Jan 22, 2014, 10:45:05 AM1/22/14
to Greg Weng, <dev-gaia@lists.mozilla.org>
The test environment and the app environment are very different, so it
makes sense to have different globals for jshint. "assert" shouldn't be
allowed in app code, and since the tests should mostly be "unit" it makes
sense to restrict the globals there too.
The use strict rules are pretty straightforward. Put it at the top of the
file or within the first IIFE if there is one.
If you are having a specific problem getting something to pass jshint, feel
free to ping me in irc (gnarf)
On Jan 22, 2014 10:10 AM, "Greg Weng" <snow...@gmail.com> wrote:
>
> (Sorry so may typos because I was writing in a hurry. Summarize it as
follow):
>
> 1. Copy the content passed JSHint from a normal JS file inside an app to
a unit test file would fail the linter.
> 2. Travis would check the test files as normal files, but because of the
different rules, they would be failed
> 3. To add test files in xfall.list seems a easier workaround, but I don't
think would solve the problem.
>
>
> 2014/1/22 Greg Weng <snow...@gmail.com>
>>
>> I've found the unit tests has different rules...
>>
>> 'use strict' in app/js can pass if it got placed as the first line, but
in test it would be failed and must be put in a anonymous function. Other
strange errors like there is that the JSHint would complain no 'window' or
'self', but it would not complain this for other files.
>>
>> The strangest thing is that if I copy the content passed the JSHint in
the normal files (not test), to a test file in the unit test directory, it
would complain, even for the same content!
>>
>> This is bothers me because when I pushed it to the Travis, these
different checking rules make my patch got blocked. And I don't want to
give up to set an entry in the xfalil.list ...
>>
>>
>> 2014/1/22 Corey Frang <gna...@gmail.com>
>>>
>>
>> http://about.me/snowmantw
>
>
>
>
> --
> Greg Weng
>
> http://about.me/snowmantw
makes sense to have different globals for jshint. "assert" shouldn't be
allowed in app code, and since the tests should mostly be "unit" it makes
sense to restrict the globals there too.
The use strict rules are pretty straightforward. Put it at the top of the
file or within the first IIFE if there is one.
If you are having a specific problem getting something to pass jshint, feel
free to ping me in irc (gnarf)
On Jan 22, 2014 10:10 AM, "Greg Weng" <snow...@gmail.com> wrote:
>
> (Sorry so may typos because I was writing in a hurry. Summarize it as
follow):
>
> 1. Copy the content passed JSHint from a normal JS file inside an app to
a unit test file would fail the linter.
> 2. Travis would check the test files as normal files, but because of the
different rules, they would be failed
> 3. To add test files in xfall.list seems a easier workaround, but I don't
think would solve the problem.
>
>
> 2014/1/22 Greg Weng <snow...@gmail.com>
>>
>> I've found the unit tests has different rules...
>>
>> 'use strict' in app/js can pass if it got placed as the first line, but
in test it would be failed and must be put in a anonymous function. Other
strange errors like there is that the JSHint would complain no 'window' or
'self', but it would not complain this for other files.
>>
>> The strangest thing is that if I copy the content passed the JSHint in
the normal files (not test), to a test file in the unit test directory, it
would complain, even for the same content!
>>
>> This is bothers me because when I pushed it to the Travis, these
different checking rules make my patch got blocked. And I don't want to
give up to set an entry in the xfalil.list ...
>>
>>
>> 2014/1/22 Corey Frang <gna...@gmail.com>
>>>
>>> _______________________________________________
>>> dev-gaia mailing list
>>> dev-...@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-gaia
>>
>>
>>
>>
>> --
>> Greg Weng
>>> dev-gaia mailing list
>>> dev-...@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-gaia
>>
>>
>>
>>
>> --
>>
>> http://about.me/snowmantw
>>
>>> Understand y f = f [ y f ] ; lose last remaining non-major friend
>>> -- Anonymous
>>> Understand y f = f [ y f ] ; lose last remaining non-major friend
>
>
>
>
> --
> Greg Weng
>
> http://about.me/snowmantw
>
>> Understand y f = f [ y f ] ; lose last remaining non-major friend
>> -- Anonymous
>> Understand y f = f [ y f ] ; lose last remaining non-major friend
Julien Wajsberg
Jan 22, 2014, 10:54:46 AM1/22/14
to Greg Weng, Corey Frang, dev-gaia
Can you show me the pull request ? :) Is it
https://github.com/mozilla-b2g/gaia/pull/15517 ?
I added specific .jshintrc files for the unit tests and for marionette
tests, as well as for some of the apps (clock, email, camera) that uses
a loader with a global "define" var.
All .jshintrc that are in unit and marionette test directories inherit
from the main .jshintrc, not from the app's .jshintrc when existing. So
that's maybe what you're seeing? I did it like this because I assumed
the app developers would know better than me how they want jshint to behave.
About 'use strict'; just use it as your first code line, whether it's
inside a IIFE or not.
Le 22/01/2014 16:06, Greg Weng a écrit :
> I've found the unit tests has different rules...
>
> 'use strict' in app/js can pass if it got placed as the first line, but in
> test it would be failed and must be put in a anonymous function. Other
> strange errors like there is that the JSHint would complain no 'window' or
> 'self', but it would not complain this for other files.
>
> The strangest thing is that if I copy the content passed the JSHint in the
> normal files (not test), to a test file in the unit test directory, it
> would complain, even for the same content!
>
> This is bothers me because when I pushed it to the Travis, these different
> checking rules make my patch got blocked. And I don't want to give up to
> set an entry in the xfalil.list ...
>
>
> 2014/1/22 Corey Frang <gna...@gmail.com>
>
https://github.com/mozilla-b2g/gaia/pull/15517 ?
I added specific .jshintrc files for the unit tests and for marionette
tests, as well as for some of the apps (clock, email, camera) that uses
a loader with a global "define" var.
All .jshintrc that are in unit and marionette test directories inherit
from the main .jshintrc, not from the app's .jshintrc when existing. So
that's maybe what you're seeing? I did it like this because I assumed
the app developers would know better than me how they want jshint to behave.
About 'use strict'; just use it as your first code line, whether it's
inside a IIFE or not.
Le 22/01/2014 16:06, Greg Weng a écrit :
> I've found the unit tests has different rules...
>
> 'use strict' in app/js can pass if it got placed as the first line, but in
> test it would be failed and must be put in a anonymous function. Other
> strange errors like there is that the JSHint would complain no 'window' or
> 'self', but it would not complain this for other files.
>
> The strangest thing is that if I copy the content passed the JSHint in the
> normal files (not test), to a test file in the unit test directory, it
> would complain, even for the same content!
>
> This is bothers me because when I pushed it to the Travis, these different
> checking rules make my patch got blocked. And I don't want to give up to
> set an entry in the xfalil.list ...
>
>
> 2014/1/22 Corey Frang <gna...@gmail.com>
>
Greg Weng
Jan 22, 2014, 11:11:02 AM1/22/14
to Julien Wajsberg, dev-gaia, Corey Frang
Thanks. I solved the problem with newer JSHint. I can't use `make hint`
because of the permission issue, so I upgrade it manually.
2014/1/22 Julien Wajsberg <jwaj...@mozilla.com>
* -- Anonymous*
because of the permission issue, so I upgrade it manually.
2014/1/22 Julien Wajsberg <jwaj...@mozilla.com>
> >> _______________________________________________
> >> dev-gaia mailing list
> >> dev-...@lists.mozilla.org
> >> https://lists.mozilla.org/listinfo/dev-gaia
> >>
> >
> >
>
>
>
*Understand y f = f [ y f ] ; lose last remaining non-major friend*
> >> dev-gaia mailing list
> >> dev-...@lists.mozilla.org
> >> https://lists.mozilla.org/listinfo/dev-gaia
> >>
> >
> >
>
>
>
* -- Anonymous*
Julien Wajsberg
Jan 22, 2014, 11:52:45 AM1/22/14
to Greg Weng, dev-gaia, Corey Frang
Great!
Actually we make use of some of the newer things in jshint so you need
an uptodate version !
"npm install -g jshint" in the gaia directory should work.
Le 22/01/2014 17:11, Greg Weng a écrit :
> Thanks. I solved the problem with newer JSHint. I can't use `make
> hint` because of the permission issue, so I upgrade it manually.
>
>
> 2014/1/22 Julien Wajsberg <jwaj...@mozilla.com
> <mailto:jwaj...@mozilla.com>>
> > 2014/1/22 Corey Frang <gna...@gmail.com <mailto:gna...@gmail.com>>
> >> _______________________________________________
> >> dev-gaia mailing list
> >> dev-...@lists.mozilla.org <mailto:dev-...@lists.mozilla.org>
> / -- Anonymous
> /
>
Actually we make use of some of the newer things in jshint so you need
an uptodate version !
"npm install -g jshint" in the gaia directory should work.
Le 22/01/2014 17:11, Greg Weng a écrit :
> Thanks. I solved the problem with newer JSHint. I can't use `make
> hint` because of the permission issue, so I upgrade it manually.
>
>
> 2014/1/22 Julien Wajsberg <jwaj...@mozilla.com
> >> dev-gaia mailing list
> >> dev-...@lists.mozilla.org <mailto:dev-...@lists.mozilla.org>
> >> https://lists.mozilla.org/listinfo/dev-gaia
> >>
> >
> >
>
>
>
>
>
> --
> <http://about.me/snowmantw>Greg Weng
>
> http://about.me/snowmantw
>
> /Understand y f = f [ y f ] ; lose last remaining non-major friend/
> >>
> >
> >
>
>
>
>
>
> --
> <http://about.me/snowmantw>Greg Weng
>
> http://about.me/snowmantw
>
> / -- Anonymous
> /
>
Anthony Ricaud
Jan 22, 2014, 11:53:46 AM1/22/14
to mozilla-...@lists.mozilla.org
I've had the same issue.
The problem is that your global JSHint was too old and doesn't support
the extends option. Run |npm install -g jshint| once to update your
global jshint and it should work. Or always run |
../node_modules/.bin/jshint|.
|make hint| is using the local jshint, so it isn't subject to this issue.
The problem is that your global JSHint was too old and doesn't support
the extends option. Run |npm install -g jshint| once to update your
global jshint and it should work. Or always run |
../node_modules/.bin/jshint|.
|make hint| is using the local jshint, so it isn't subject to this issue.
Mike Pennisi
Jan 22, 2014, 12:04:07 PM1/22/14
to dev-...@lists.mozilla.org
Since this whole workflow is mediated by a Makefile, is it necessary to
install the module globally? Globally-installed tools can be challenging
for developers working with multiple projects. Installing locally also
lets us control the version of the linter locally via Gaia's
`package.json` file--no need to ask developers to manually update their
installation.
If there is nothing about the infrastructure that would technically
prevent this approach, I'd be happy to file a follow-up bug to make this
change.
Mike
On 01/22/2014 11:52 AM, Julien Wajsberg wrote:
> Great!
>
> Actually we make use of some of the newer things in jshint so you need
> an uptodate version !
>
> "npm install -g jshint" in the gaia directory should work.
>
> Le 22/01/2014 17:11, Greg Weng a écrit :
>> > 2014/1/22 Corey Frang <gna...@gmail.com <mailto:gna...@gmail.com>>
>> >> _______________________________________________
>> >> dev-gaia mailing list
install the module globally? Globally-installed tools can be challenging
for developers working with multiple projects. Installing locally also
lets us control the version of the linter locally via Gaia's
`package.json` file--no need to ask developers to manually update their
installation.
If there is nothing about the infrastructure that would technically
prevent this approach, I'd be happy to file a follow-up bug to make this
change.
Mike
On 01/22/2014 11:52 AM, Julien Wajsberg wrote:
> Great!
>
> Actually we make use of some of the newer things in jshint so you need
> an uptodate version !
>
> "npm install -g jshint" in the gaia directory should work.
>
> Le 22/01/2014 17:11, Greg Weng a écrit :
>> Thanks. I solved the problem with newer JSHint. I can't use `make
>> hint` because of the permission issue, so I upgrade it manually.
>>
>>
>> 2014/1/22 Julien Wajsberg <jwaj...@mozilla.com
>> <mailto:jwaj...@mozilla.com>>
>> hint` because of the permission issue, so I upgrade it manually.
>>
>>
>> 2014/1/22 Julien Wajsberg <jwaj...@mozilla.com
>> >> dev-gaia mailing list
>> >> dev-...@lists.mozilla.org <mailto:dev-...@lists.mozilla.org>
>> >> https://lists.mozilla.org/listinfo/dev-gaia
>> >>
>> >
>> >
>>
>>
>>
>>
>>
>> --
>> <http://about.me/snowmantw>Greg Weng
>>
>> http://about.me/snowmantw
>>
>> /Understand y f = f [ y f ] ; lose last remaining non-major friend/
>> / -- Anonymous
>> /
>>
>
>
>> >> https://lists.mozilla.org/listinfo/dev-gaia
>> >>
>> >
>> >
>>
>>
>>
>>
>>
>> --
>> <http://about.me/snowmantw>Greg Weng
>>
>> http://about.me/snowmantw
>>
>> /Understand y f = f [ y f ] ; lose last remaining non-major friend/
>> / -- Anonymous
>> /
>>
>
>
Julien Wajsberg
Jan 22, 2014, 12:06:38 PM1/22/14
to Mike Pennisi, dev-...@lists.mozilla.org
Le 22/01/2014 18:04, Mike Pennisi a écrit :
> Since this whole workflow is mediated by a Makefile, is it necessary
> to install the module globally? Globally-installed tools can be
> challenging for developers working with multiple projects. Installing
> locally also lets us control the version of the linter locally via
> Gaia's `package.json` file--no need to ask developers to manually
> update their installation.
>
> If there is nothing about the infrastructure that would technically
> prevent this approach, I'd be happy to file a follow-up bug to make
> this change.
This is not needed for "make hint" or the commit hook, they are using
> Since this whole workflow is mediated by a Makefile, is it necessary
> to install the module globally? Globally-installed tools can be
> challenging for developers working with multiple projects. Installing
> locally also lets us control the version of the linter locally via
> Gaia's `package.json` file--no need to ask developers to manually
> update their installation.
>
> If there is nothing about the infrastructure that would technically
> prevent this approach, I'd be happy to file a follow-up bug to make
> this change.
the version controlled by package.json, which is automatically installed
when running "make hint".
But for someone running "jshint" manually, or using their editor's
plugin, it's using the globally installed jshint, and that's what I was
refering to.
The safest solution is actually to use "./node_modules/.bin/jshint" as
Rik said, maybe make it an alias if necessary.
--
Julien
James Burke
Jan 22, 2014, 1:03:32 PM1/22/14
to Julien Wajsberg, dev-...@lists.mozilla.org
On Wed, Jan 22, 2014 at 1:57 AM, Julien Wajsberg <jwaj...@mozilla.com> wrote:
> * then, there is a file (called xfail.list) which lists all files that
> currently fail jshint. I think it would be merged in with the failed
> files, right ? And then their errors would be ignored.
Ah, yes, xfail.list. Since it already contains the files on master
> * then, there is a file (called xfail.list) which lists all files that
> currently fail jshint. I think it would be merged in with the failed
> files, right ? And then their errors would be ignored.
that do not pass, it will avoid the merge issue I described. Thanks,
and hooray to Corey and Julien for getting this landed!
Kevin Grandon
Jan 23, 2014, 12:56:33 AM1/23/14
to James Burke, Julien Wajsberg, dev-...@lists.mozilla.org
Important: If you have a pull request open from before this went live, please rebase against master and ensure a green travis run before landing.
We're seeing lots of travis failures with this today. I've already backed out two commits, so hopefully we won't have to back out any more.
Best,
Kevin
We're seeing lots of travis failures with this today. I've already backed out two commits, so hopefully we won't have to back out any more.
Best,
Kevin
0 new messages