threads / patch / 39765

patchrebase: return non-zero error code if format-patch fails

Subject: [PATCH] rebase: return non-zero error code if format-patch fails

## tl;dr

4 messages between Jul 2, 2015 and Jul 6, 2015. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Clemens Buchacher· Jul 2, 2015, 09:11 UTC · lore

Since e481af06 (rebase: Handle cases where format-patch fails) we notice if format-patch fails and return immediately from git-rebase--am. We save the return value with ret=$?, but then we return $?, which is usually zero in this case.

Fix this by returning $ret instead.
Cc: Andrew Wong <andrew.kw.w@gmail.com>
Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com>
Reviewed-by: Jorge Nunes <jorge.nunes@intel.com>
---
 git-rebase--am.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to git-rebase--am.sh +1 −1
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index f923732..9ae898b 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -78,7 +78,7 @@ else
 
 		As a result, git cannot rebase them.
 		EOF
-		return $?
+		return $ret
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
-- 
1.9.4
Junio C Hamano· Jul 3, 2015, 17:52 UTC · re: Clemens Buchacher · lore

Re: [PATCH] rebase: return non-zero error code if format-patch fails

Clemens Buchacher <clemens.buchacher@intel.com> writes:
Show 6 quoted lines
> Since e481af06 (rebase: Handle cases where format-patch fails) we
> notice if format-patch fails and return immediately from
> git-rebase--am. We save the return value with ret=$?, but then we
> return $?, which is usually zero in this case.
>
> Fix this by returning $ret instead.
Sounds sensible.
>
> Cc: Andrew Wong <andrew.kw.w@gmail.com>
> Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com>
> Reviewed-by: Jorge Nunes <jorge.nunes@intel.com>

Where was this review made? I may have missed a recent discussion, and that is why I am asking, because Reviewed-by: lines that cannot be validated by going back to the list archive does not add much value.

Thanks.
Show 17 quoted lines
> ---
>  git-rebase--am.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index f923732..9ae898b 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -78,7 +78,7 @@ else
>  
>  		As a result, git cannot rebase them.
>  		EOF
> -		return $?
> +		return $ret
>  	fi
>  
>  	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
Clemens Buchacher· Jul 6, 2015, 08:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase: return non-zero error code if format-patch fails

On Fri, Jul 03, 2015 at 10:52:32AM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> >
> > Cc: Andrew Wong <andrew.kw.w@gmail.com>
> > Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com>
> > Reviewed-by: Jorge Nunes <jorge.nunes@intel.com>
> 
> Where was this review made?  I may have missed a recent discussion,
> and that is why I am asking, because Reviewed-by: lines that cannot
> be validated by going back to the list archive does not add much
> value.

Jorge helped me by reviewing the patch before I submitted it to the list. My intention is to give credit for his contribution, and to involve him in any discussion regarding the patch. Maybe it makes more sense to say Helped-by:? Please feel free to change as you see fit. I will follow your recommendation in the future.

Thanks.
Junio C Hamano· Jul 6, 2015, 17:01 UTC · re: Clemens Buchacher · lore

Re: [PATCH] rebase: return non-zero error code if format-patch fails

Clemens Buchacher <clemens.buchacher@intel.com> writes:
Show 15 quoted lines
> On Fri, Jul 03, 2015 at 10:52:32AM -0700, Junio C Hamano wrote:
>> >
>> > Cc: Andrew Wong <andrew.kw.w@gmail.com>
>> > Signed-off-by: Clemens Buchacher <clemens.buchacher@intel.com>
>> > Reviewed-by: Jorge Nunes <jorge.nunes@intel.com>
>> 
>> Where was this review made?  I may have missed a recent discussion,
>> and that is why I am asking, because Reviewed-by: lines that cannot
>> be validated by going back to the list archive does not add much
>> value.
>
> Jorge helped me by reviewing the patch before I submitted it to the
> list. My intention is to give credit for his contribution, and to
> involve him in any discussion regarding the patch. Maybe it makes more
> sense to say Helped-by:?

Thanks; I think that clarifies it, and I think that is how people seem to use Helped-by around here.

← back to recent threads