Closed
Bug 1316925
Opened 8 years ago
Closed 8 years ago
"./mach eslint --setup" should not print out "0 errors, 0 warnings", if it fails & prints a warning/error about failing
Categories
(Developer Infrastructure :: Lint and Formatting, defect)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: dholbert, Assigned: ahal)
Details
Attachments
(1 file)
STR:
(1) Don't have nodejs installed
(2) Run ./mach eslint --setup
ACTUAL RESULTS:
Something like the following:
> nodejs v4.2.3 is either not installed or is installed to
> a non-standard path.
> Please install nodejs from https://nodejs.org and try again.
>
> Valid installation paths:
> - /usr/bin/nodejs
> ✖ 0 problems (0 errors, 0 warnings)
Note the last line in particular, which is very confusing in light of the error/warning directly before it.
EXPECTED RESULTS:
That "0 problems (0 errors, 0 warnings)" line should not be printed if we failed to find node (or failed to run/setup eslint due to other reasons).
Reporter | ||
Updated•8 years ago
|
Summary: "./mach eslint --setup" should not print out "0 errors, 0 warnings" if it fails due to an error → "./mach eslint --setup" should not print out "0 errors, 0 warnings", if it fails & prints a warning/error about failing
Assignee | ||
Comment 1•8 years ago
|
||
Agree it's confusing. Fwiw, it's a bit more nuanced because you could be running multiple linters at a time, some of which error out, and others which run successfully. I think a good solution is to count a non-zero return code as a "problem".
So in the case something goes wrong you might see:
✖ 1 problem (0 errors, 0 warnings, 1 exception)
Reporter | ||
Comment 2•8 years ago
|
||
That output would've made more sense to me, yeah.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8810583 [details]
Bug 1316925 - Keep track of failed linters in stylish formatter summary,
https://reviewboard.mozilla.org/r/92866/#review94176
::: python/mozlint/mozlint/cli.py:109
(Diff revision 1)
>
> # Explicitly utf-8 encode the output as some of the formatters make
> # use of unicode characters. This will prevent a UnicodeEncodeError
> # on environments where utf-8 isn't the default
> - print(formatter(results).encode('utf-8', 'replace'))
> - return lint.return_code
> + print(formatter(results, failed=lint.failed).encode('utf-8', 'replace'))
> + return 1 if results or lint.failed else 0
I might be tempted to have different return codes for the two cases (failure vs lint errors) but not that important.
Attachment #8810583 -
Flags: review?(james) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/127e45e5ba1d
Keep track of failed linters in stylish formatter summary, r=jgraham
Comment 6•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•7 years ago
|
Product: Testing → Firefox Build System
Updated•6 years ago
|
Version: Version 3 → 3 Branch
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•