Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Phillip Wood]

On 18/11/17 03:57, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Phillip Wood  writes:
>>
>>> From: Phillip Wood 
>>>
>>> I've updated these based on the feedback for v2. I've dropped the
>>> patch that stopped print_commit_summary() from dying as I think it is
>>> better to die than return an error (see the commit message of the
>>> patch that adds print_commit_summary() for the reasoning). Apart from
>>> that they're minor changes - style fixes and a reworded a commit message.
>>
>> Thanks for further polishing this topic; I found nothing in the
>> update that was questionable.  Will replace.
>>
>> With this, perhaps it is ready for 'next'?
> 
> Not really.  I needed at least this to get it even compile, which
> hints that I do not yet know what _else_ I missed by skimming this
> round of the series.

My apologies, it seems that I was half alseep yesterday morning and in
my haste to update these patches I wasn't paying proper attention to
what I was doing. For some reason after I dropped the patch that
converted print_commit_summary() to return an error rather than die I
forgot to amend the last patch to match and then compounded the mistake
by forgetting to compile and test properly before sending them. I think
they're okay now but I'll double check the changes before sending again
in case there are any other embarrassing mistakes lurking.

As the white rabbit said "The hurrier I go the behinder I get"

Phillip


>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 37460db6b1..63cfb6ddd9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char 
> *author,
>   unlink(git_path_cherry_pick_head());
>   unlink(git_path_merge_msg());
>   if (!is_rebase_i(opts))
> - res = print_commit_summary(NULL, ,
> - SUMMARY_SHOW_AUTHOR_DATE);
> + print_commit_summary(NULL, ,
> +  SUMMARY_SHOW_AUTHOR_DATE);
>   return res;
>   }
>   }
> 

Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Phillip Wood]

On 18/11/17 03:57, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Phillip Wood  writes:
>>
>>> From: Phillip Wood 
>>>
>>> I've updated these based on the feedback for v2. I've dropped the
>>> patch that stopped print_commit_summary() from dying as I think it is
>>> better to die than return an error (see the commit message of the
>>> patch that adds print_commit_summary() for the reasoning). Apart from
>>> that they're minor changes - style fixes and a reworded a commit message.
>>
>> Thanks for further polishing this topic; I found nothing in the
>> update that was questionable.  Will replace.
>>
>> With this, perhaps it is ready for 'next'?
> 
> Not really.  I needed at least this to get it even compile, which
> hints that I do not yet know what _else_ I missed by skimming this
> round of the series.

Sorry I'm not sure what happened there, by branch has the missing 'res =
' something must have happened to the patches. I'll sort it out and resend

Phillip

>  sequencer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 37460db6b1..63cfb6ddd9 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char 
> *author,
>   unlink(git_path_cherry_pick_head());
>   unlink(git_path_merge_msg());
>   if (!is_rebase_i(opts))
> - res = print_commit_summary(NULL, ,
> - SUMMARY_SHOW_AUTHOR_DATE);
> + print_commit_summary(NULL, ,
> +  SUMMARY_SHOW_AUTHOR_DATE);
>   return res;
>   }
>   }
> 

Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Junio C Hamano]

Junio C Hamano  writes:

> Phillip Wood  writes:
>
>> From: Phillip Wood 
>>
>> I've updated these based on the feedback for v2. I've dropped the
>> patch that stopped print_commit_summary() from dying as I think it is
>> better to die than return an error (see the commit message of the
>> patch that adds print_commit_summary() for the reasoning). Apart from
>> that they're minor changes - style fixes and a reworded a commit message.
>
> Thanks for further polishing this topic; I found nothing in the
> update that was questionable.  Will replace.
>
> With this, perhaps it is ready for 'next'?

Not really.  I needed at least this to get it even compile, which
hints that I do not yet know what _else_ I missed by skimming this
round of the series.

 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 37460db6b1..63cfb6ddd9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1139,8 +1139,8 @@ static int do_commit(const char *msg_file, const char 
*author,
unlink(git_path_cherry_pick_head());
unlink(git_path_merge_msg());
if (!is_rebase_i(opts))
-   res = print_commit_summary(NULL, ,
-   SUMMARY_SHOW_AUTHOR_DATE);
+   print_commit_summary(NULL, ,
+SUMMARY_SHOW_AUTHOR_DATE);
return res;
}
}
-- 
2.15.0-372-g9a6f8facfd

Re: [PATCH v3 0/8] sequencer: don't fork git commit

[Junio C Hamano]

Phillip Wood  writes:

> From: Phillip Wood 
>
> I've updated these based on the feedback for v2. I've dropped the
> patch that stopped print_commit_summary() from dying as I think it is
> better to die than return an error (see the commit message of the
> patch that adds print_commit_summary() for the reasoning). Apart from
> that they're minor changes - style fixes and a reworded a commit message.

Thanks for further polishing this topic; I found nothing in the
update that was questionable.  Will replace.

With this, perhaps it is ready for 'next'?