[eslint] Don't treat eslint warnings as PRESUBMIT errors

[eslint] Don't treat eslint warnings as PRESUBMIT errors

This CL makes it so that any warnings emitted by eslint will not cause the
PRESUBMIT to error out. In the process of doing so, it cleans up the structure
of the eslint Python module; we were using a find+xargs-based method of linting
directories in order to find .html files, which is not necessary given eslint's
--ext option.

BUG=catapult:#3034

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/355ca2541b19167eea565d3cad751c3f8c26db51

[eakuefner, 2016-11-28 21:31:30]

eakuefner@chromium.org changed reviewers:
+ charliea@chromium.org

[eakuefner, 2016-11-28 21:31:30]

PTAL

[charliea (OOO until 10-5), 2016-11-28 21:54:21]

https://codereview.chromium.org/2535983002/diff/1/common/eslint/bin/run_eslint
File common/eslint/bin/run_eslint (right):

https://codereview.chromium.org/2535983002/diff/1/common/eslint/bin/run_eslin...
common/eslint/bin/run_eslint:37: parser.add_argument('--files', '-f',
default=None, nargs='+', metavar='FILE',
(Here and wherever makes sense) can you change "file" to "path" to make  it more
accurate?

https://codereview.chromium.org/2535983002/diff/1/common/eslint/eslint/__init...
File common/eslint/eslint/__init__.py (right):

https://codereview.chromium.org/2535983002/diff/1/common/eslint/eslint/__init...
common/eslint/eslint/__init__.py:52: def RunEslint(filenames,
rules_dir=DEFAULT_ESLINT_RULES_DIR, extra_args=None):
For the return value:

- I think it might make sense to switch it so that were_errors is first
- I also think it might make sense to reverse the meaning of were_errors so that
instead it's a success boolean

Then, someone could plausibly do the normal thing of:

if not eslint.RunEslint(...):
  # Handle error case...
  
if they wanted to

[eakuefner, 2016-11-28 22:27:50]

Thanks for your comments; please take another look.

https://codereview.chromium.org/2535983002/diff/1/common/eslint/bin/run_eslint
File common/eslint/bin/run_eslint (right):

https://codereview.chromium.org/2535983002/diff/1/common/eslint/bin/run_eslin...
common/eslint/bin/run_eslint:37: parser.add_argument('--files', '-f',
default=None, nargs='+', metavar='FILE',
On 2016/11/28 at 21:54:21, charliea wrote:
> (Here and wherever makes sense) can you change "file" to "path" to make  it
more accurate?

Done.

https://codereview.chromium.org/2535983002/diff/1/common/eslint/eslint/__init...
File common/eslint/eslint/__init__.py (right):

https://codereview.chromium.org/2535983002/diff/1/common/eslint/eslint/__init...
common/eslint/eslint/__init__.py:52: def RunEslint(filenames,
rules_dir=DEFAULT_ESLINT_RULES_DIR, extra_args=None):
On 2016/11/28 at 21:54:21, charliea wrote:
> For the return value:
> 
> - I think it might make sense to switch it so that were_errors is first
> - I also think it might make sense to reverse the meaning of were_errors so
that instead it's a success boolean
> 
> Then, someone could plausibly do the normal thing of:
> 
> if not eslint.RunEslint(...):
>   # Handle error case...
>   
> if they wanted to

Done.

[charliea (OOO until 10-5), 2016-11-29 19:29:18]

lgtm

[charliea (OOO until 10-5), 2016-11-29 19:32:13]

The CQ bit was checked by charliea@chromium.org

[commit-bot: I haz the power, 2016-11-29 19:32:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 19:34:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-29 19:34:49]

Try jobs failed on following builders:
  Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Pr...)

[eakuefner, 2016-11-29 22:24:13]

eakuefner@chromium.org changed reviewers:
+ sullivan@chromium.org

[eakuefner, 2016-11-29 22:24:13]

+Annie for Catapult OWNERS

[sullivan, 2016-11-29 23:00:50]

lgtm

[eakuefner, 2016-11-29 23:01:29]

The CQ bit was checked by eakuefner@chromium.org

[commit-bot: I haz the power, 2016-11-29 23:01:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-29 23:06:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-29 23:06:21]

Try jobs failed on following builders:
  Catapult Linux Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Li...)

[eakuefner, 2016-12-05 19:36:56]

The CQ bit was checked by eakuefner@chromium.org

[eakuefner, 2016-12-05 19:36:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
sullivan@chromium.org, charliea@chromium.org
Link to the patchset: https://codereview.chromium.org/2535983002/#ps40001
(title: "fix smoke test")

[commit-bot: I haz the power, 2016-12-05 19:37:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 20:03:17]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480966616006980,
"parent_rev": "750e652668fcfb915d77007f14c5b6befb3a704e", "commit_rev":
"355ca2541b19167eea565d3cad751c3f8c26db51"}

[commit-bot: I haz the power, 2016-12-05 20:03:20]

Description was changed from

==========
[eslint] Don't treat eslint warnings as PRESUBMIT errors

This CL makes it so that any warnings emitted by eslint will not cause the
PRESUBMIT to error out. In the process of doing so, it cleans up the structure
of the eslint Python module; we were using a find+xargs-based method of linting
directories in order to find .html files, which is not necessary given eslint's
--ext option.

BUG=catapult:#3034
==========

to

==========
[eslint] Don't treat eslint warnings as PRESUBMIT errors

This CL makes it so that any warnings emitted by eslint will not cause the
PRESUBMIT to error out. In the process of doing so, it cleans up the structure
of the eslint Python module; we were using a find+xargs-based method of linting
directories in order to find .html files, which is not necessary given eslint's
--ext option.

BUG=catapult:#3034

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-12-05 20:03:20]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...