Re: [PATCH] git-p4: Handle p4 submit failure
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:06
Etienne Girard [off-list ref] writes:
Clean the workspace if p4_write_pipe raised SystemExit, so that the user don't have to do it themselves. Signed-off-by: GIRARD Etienne <redacted> ---
Thanks. As I do not really do "p4", I am CC'ing this response to
the area experts.
First, one "procedural" comment.
You sent the patch from your gmail.com address, but signed it off
with another address (with name spelled differently). If applied
as-is, the result will have your gmail.com address listed as the
author and that will be the name you would see in "git shortlog".
If you rather prefer to be known as the name and address you placed
on your sign-off, while sending the patch from another address, you
can do so by staring the _body_ of the message with what is called
an "in-body header", like this:
From: GIRARD Etienne [off-list ref]
Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.
Signed-off-by: GIRARD Etienne [off-list ref]
---
... here you have the usual patch ...
That "From:" line would sit on the first line of the body of your
message, separated by a blank line from the body of the log message.
This is not even a problem from the project's point of view, but I
thought you would want to be aware of it, and may want a chance to
change it.
Note: this time, you do not need to resend the patch; just
please let me know if you want me to do the equivalent of
the above while applying to make your murex address and name
appear as the author in "git log" and "git shortlog" output.
FYI, you could override the "Subject:" of your e-mail header with an
in-body header (this is often useful when replying to an existing
discussion thread with a patch that shows a solution), e.g.
From: GIRARD Etienne [off-list ref]
Subject: git-p4: clean up after p4 submit failure
Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.
Signed-off-by: GIRARD Etienne [off-list ref]
---
... here you have the usual patch ...
git-p4.py | 71 +++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 34 deletions(-) The p4 submit command may fail, for example if the changelist contains a job that does not exist in the Jobs section. If this is the case, p4_write_pipe will exit the script. This change makes it so that the workspace is reverted before letting the script die.
Some of the information contained in this paragraph deserves to be
in the log message proper. How about
From: GIRARD Etienne [off-list ref]
Subject: git-p4: clean up after p4 submit failure
When "p4 submit" command fails in P4Submit.applyCommit, the
workspace is left with the changes. We already have a code
to revert the changes to the workspace when the user decides
to cancel submission by aborting the editor that edits the
change description, and we should treat the "p4 submit"
failure the same way.
Clean the workspace if p4_write_pipe raised SystemExit,
so that the user don't have to do it themselves.
Signed-off-by: GIRARD Etienne [off-list ref]
or something like that?
While trying to come up with the above description, I started
wondering if the error message wants to differentiate the two cases.
When self.edit_template() returns false, we know that the user
actively said "no I do not want to submit", and "Submission
cancelled" is perfectly fine, but when "p4 submit" fails because it
did not like the change description or if it found some other
issues, it is not necessarily that the user said "I want to cancel";
it is more like "Submission failed".
Hmm?