On 10/14/2019 11:38 AM, James Coglan wrote:
> On 13/10/2019 07:56, Jeff King wrote:
>> On Fri, Oct 11, 2019 at 06:04:21PM +0100, James Coglan wrote:
>>
>>>> I should have noticed in your earlier commits, but why don't you keep
>>>> the output inside the test suite? You can start with "cat >expect <<-EOF"
>>>> to have it ignore leading whitespace. Sorry if there's something else about
>>>> this that is causing issues.
>>>
>>> I was following a pattern used in t/t4202-log.sh. I believe it was
>>> easier to debug these tests with the setup and expectations split into
>>> separate blocks, but I wouldn't be opposed to merging them.
>>
>> Some of the older tests used that style, but we've been slowly
>> modernizing (I know, it's hard to pick up the style by example in such
>> cases!). The usual style these days is making sure everything goes in a
>> test_expect_* block, with "<<-" to indent here-documents.
>>
>> Another minor style nit that you picked up from t4202:
>>
>>>>> +cat > expect <<\EOF
>>
>> We'd omit the space after ">" here.
> 
> Thanks, I've now made the style changes you've suggested on my branch. How 
> should I go about sharing the current state of my patch series after I've 
> incorporated all the changes suggested here? Should I post them as replies on 
> this thread, or start a new pull request via GitGitGadget?

Since you sent v1 via GitGitGadget, all you need to do is
add another "/submit" comment on the same PR and it will
send a v2 to this thread.

It will auto-generate the range-diff from v1 and append it
to your cover letter.

-Stolee

Reply via email to