mbox series

[v2,0/5] am/rebase: share read_author_script()

Message ID 20181018100023.7327-1-phillip.wood@talktalk.net (mailing list archive)
Headers show
Series am/rebase: share read_author_script() | expand

Message

Phillip Wood Oct. 18, 2018, 10 a.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Thanks to Eric for his feedback on v1. I've rerolled based on
that. Patches 1 & 2 are new and try to address some of the concerns
Eric raised, particularly the error handling for a badly edited author
script. See the notes on patches 4 & 5 for the changes to those (they
were previously 2 & 3).

v1 cover letter:

This is a follow up to pw/rebase-i-author-script-fix, it reduces code
duplication and improves rebase's parsing of the author script. After
this I'll do another series to share the code to write the author
script.

Phillip Wood (5):
  am: don't die in read_author_script()
  am: improve author-script error reporting
  am: rename read_author_script()
  add read_author_script() to libgit
  sequencer: use read_author_script()

 builtin/am.c |  60 ++--------------
 sequencer.c  | 192 ++++++++++++++++++++++++++++++++-------------------
 sequencer.h  |   3 +
 3 files changed, 128 insertions(+), 127 deletions(-)

Comments

Junio C Hamano Oct. 25, 2018, 8:59 a.m. UTC | #1
Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Thanks to Eric for his feedback on v1. I've rerolled based on
> that. Patches 1 & 2 are new and try to address some of the concerns
> Eric raised, particularly the error handling for a badly edited author
> script. See the notes on patches 4 & 5 for the changes to those (they
> were previously 2 & 3).

I spotted a weird corner case buglet, but it seems that this one is
ready for 'next' even without fixing that "give it three times and
we will happily continue" thing.

Do we know of any other issues?  Can we now move it forward?

Thanks.
Phillip Wood Oct. 26, 2018, 9 a.m. UTC | #2
Hi Junio

On 25/10/2018 09:59, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> Thanks to Eric for his feedback on v1. I've rerolled based on
>> that. Patches 1 & 2 are new and try to address some of the concerns
>> Eric raised, particularly the error handling for a badly edited author
>> script. See the notes on patches 4 & 5 for the changes to those (they
>> were previously 2 & 3).
> 
> I spotted a weird corner case buglet, but it seems that this one is
> ready for 'next' even without fixing that "give it three times and
> we will happily continue" thing.

Well spotted on the corner case. If you're happy to hold off on moving
it to next I can send a re-roll with fixes for that next week or I can
do it as a fixup if you want to move this forward now.

> Do we know of any other issues?  Can we now move it forward?

I don't know of any other issues but I don't think anyone else has
looked at this iteration. Thanks for taking a look at these.

Best Wishes

Phillip



> Thanks.
>
Junio C Hamano Oct. 26, 2018, 1:32 p.m. UTC | #3
Phillip Wood <phillip.wood@talktalk.net> writes:

>> I spotted a weird corner case buglet, but it seems that this one is
>> ready for 'next' even without fixing that "give it three times and
>> we will happily continue" thing.
>
> Well spotted on the corner case. If you're happy to hold off on moving
> it to next I can send a re-roll with fixes for that next week or I can
> do it as a fixup if you want to move this forward now.

OK, I'll mark it as "expecting a reroll".