1375834 - [mozlint] Stop printing command if vcs failed

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Andrew Halberstadt [:ahal]]

In bug 1369787 I started printing the version control command used if the command returned non-zero. But when using ./mach lint --outgoing, hg returns non-zero if no files were found. Printing the command in this case feels unnatural and unnecessary.

I think we should revert to the previous behaviour of only printing the error output (if it exists).

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1375834 - [mozlint] Stop printing vcs command used if command returns non-zero,

Review commit: https://reviewboard.mozilla.org/r/152164/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152164/

Comment 2

[Bob Clary [:bc] (inactive)]

Comment on attachment 8880790 [details]
Bug 1375834 - [mozlint] Stop printing vcs command used if command returns non-zero,

https://reviewboard.mozilla.org/r/152164/#review157166

Please add the blank line after the license comment. The issue with printing the command with the error output, I'll leave up to you.

::: python/mozlint/mozlint/roller.py
(Diff revision 1)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> -

Why remove this line? All of the other files in this directory have an empty line following the license comment.

::: python/mozlint/mozlint/vcs.py:42
(Diff revision 1)
>      def run(self, cmd):
>          try:
>              files = subprocess.check_output(cmd, stderr=subprocess.STDOUT).split()
>          except subprocess.CalledProcessError as e:
> -            print(' '.join(cmd))
> +            if e.output:
> -            print(e.output)
> +                print(e.output)

You don't think it would be useful to print the command along with the error output ? It would eliminate the printing of the command if there was no output which is the problematic change you would like to revert, right?

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8880790 [details]
Bug 1375834 - [mozlint] Stop printing vcs command used if command returns non-zero,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152164/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0db1768c1509
[mozlint] Stop printing vcs command used if command returns non-zero, r=bc

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0db1768c1509