[U-Boot] [PATCH] patman: make run results better visible

For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org ---
tools/patman/patman.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index c60aa5a..0163ccd 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -154,13 +154,19 @@ else:
# Email the patches out (giving the user time to check / cancel) cmd = '' - if ok or options.ignore_errors: + its_a_go = ok or options.ignore_errors + if its_a_go: cmd = gitutil.EmailPatches(series, cover_fname, args, options.dry_run, not options.ignore_bad_tags, cc_file, in_reply_to=options.in_reply_to) + else: + print col.Color(col.RED, + "Not sending emails due to checkpatch errors/warnings")
# For a dry run, just show our actions as a sanity check if options.dry_run: series.ShowActions(args, cmd, options.process_tags) + if not its_a_go: + print col.Color(col.RED, "Email would not be sent")
os.remove(cc_file)

Vadim,
On Wed, Sep 3, 2014 at 12:16 PM, Vadim Bendebury vbendeb@chromium.org wrote:
For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org
tools/patman/patman.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index c60aa5a..0163ccd 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -154,13 +154,19 @@ else:
# Email the patches out (giving the user time to check / cancel) cmd = ''
- if ok or options.ignore_errors:
- its_a_go = ok or options.ignore_errors
- if its_a_go: cmd = gitutil.EmailPatches(series, cover_fname, args, options.dry_run, not options.ignore_bad_tags, cc_file, in_reply_to=options.in_reply_to)
- else:
print col.Color(col.RED,
"Not sending emails due to checkpatch errors/warnings")
Technically it could be due to other problems, too (like errors applying).
# For a dry run, just show our actions as a sanity check if options.dry_run: series.ShowActions(args, cmd, options.process_tags)
if not its_a_go:
print col.Color(col.RED, "Email would not be sent")
os.remove(cc_file)
Looks good to me, other than that.
-Doug

On Wed, Sep 3, 2014 at 3:14 PM, Doug Anderson dianders@chromium.org wrote:
Vadim,
On Wed, Sep 3, 2014 at 12:16 PM, Vadim Bendebury vbendeb@chromium.org wrote:
For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org
tools/patman/patman.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index c60aa5a..0163ccd 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -154,13 +154,19 @@ else:
# Email the patches out (giving the user time to check / cancel) cmd = ''
- if ok or options.ignore_errors:
- its_a_go = ok or options.ignore_errors
- if its_a_go: cmd = gitutil.EmailPatches(series, cover_fname, args, options.dry_run, not options.ignore_bad_tags, cc_file, in_reply_to=options.in_reply_to)
- else:
print col.Color(col.RED,
"Not sending emails due to checkpatch errors/warnings")
Technically it could be due to other problems, too (like errors applying).
good point, what wording would you suggest?
--vb
# For a dry run, just show our actions as a sanity check if options.dry_run: series.ShowActions(args, cmd, options.process_tags)
if not its_a_go:
print col.Color(col.RED, "Email would not be sent")
os.remove(cc_file)
Looks good to me, other than that.
-Doug

Vadim,
On Wed, Sep 3, 2014 at 4:00 PM, Vadim Bendebury vbendeb@chromium.org wrote:
On Wed, Sep 3, 2014 at 3:14 PM, Doug Anderson dianders@chromium.org wrote:
Vadim,
On Wed, Sep 3, 2014 at 12:16 PM, Vadim Bendebury vbendeb@chromium.org wrote:
For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org
tools/patman/patman.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index c60aa5a..0163ccd 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -154,13 +154,19 @@ else:
# Email the patches out (giving the user time to check / cancel) cmd = ''
- if ok or options.ignore_errors:
- its_a_go = ok or options.ignore_errors
- if its_a_go: cmd = gitutil.EmailPatches(series, cover_fname, args, options.dry_run, not options.ignore_bad_tags, cc_file, in_reply_to=options.in_reply_to)
- else:
print col.Color(col.RED,
"Not sending emails due to checkpatch errors/warnings")
Technically it could be due to other problems, too (like errors applying).
good point, what wording would you suggest?
You don't think that just removing the word "checkpatch" is enough.

For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org ---
Changes in v2: - modified the error message for accuracy
tools/patman/patman.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index c60aa5a..86e8e63 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -154,13 +154,18 @@ else:
# Email the patches out (giving the user time to check / cancel) cmd = '' - if ok or options.ignore_errors: + its_a_go = ok or options.ignore_errors + if its_a_go: cmd = gitutil.EmailPatches(series, cover_fname, args, options.dry_run, not options.ignore_bad_tags, cc_file, in_reply_to=options.in_reply_to) + else: + print col.Color(col.RED, "Not sending emails due to errors/warnings")
# For a dry run, just show our actions as a sanity check if options.dry_run: series.ShowActions(args, cmd, options.process_tags) + if not its_a_go: + print col.Color(col.RED, "Email would not be sent")
os.remove(cc_file)

Vadim,
On Thu, Sep 4, 2014 at 10:45 AM, Vadim Bendebury vbendeb@chromium.org wrote:
For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org
Changes in v2:
- modified the error message for accuracy
tools/patman/patman.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Doug Anderson dianders@chromium.org

On 4 September 2014 12:57, Doug Anderson dianders@chromium.org wrote:
Vadim,
On Thu, Sep 4, 2014 at 10:45 AM, Vadim Bendebury vbendeb@chromium.org wrote:
For an occasional user of patman some failures are not obvious: for instance when checkpatch reports warnings, the dry run still reports that the email would be sent. If it is not dry run, the warnings are shown on the screen, but it is not clear that the email was not sent.
Add some code to report failure to send email explicitly.
Tested by running the script on a patch with style violations, observed error messages in the script output.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org
Changes in v2:
- modified the error message for accuracy
tools/patman/patman.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Doug Anderson dianders@chromium.org
Acked-by: Simon Glass sjg@chromium.org

Applied to u-boot-x86/buildman, thanks!
participants (3)
-
Doug Anderson
-
Simon Glass
-
Vadim Bendebury