Re: [PATCH v3 04/10] sequencer: use the trailer iterator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Arver <linusa@xxxxxxxxxx> writes:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> "Linus Arver via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>>
>>> From: Linus Arver <linusa@xxxxxxxxxx>
>>>
>>> Also, teach the iterator about non-trailer lines, by adding a new field
>>> called "raw" to hold both trailer and non-trailer lines. This is
>>> necessary because a "trailer block" is a list of trailer lines of at
>>> least 25% trailers (see 146245063e (trailer: allow non-trailers in
>>> trailer block, 2016-10-21)), such that it may hold non-trailer lines.
>>
>> That sounds like a task larger than something we would want in a
>> patch that focuses on another task (e.g. update sequencer not to
>> call trailer_info_get()) while at it.  It seems from a casual glance
>> that the change to shortlog.c is to accomodate this change in the
>> semantics of what the iterator could return?  It smells that this
>> patch does two more or less unrelated things at the same time?
>
> I think you're correct. Hopefully breaking this up will make things
> easier to review.

And now that I've broken it up locally, I can say that the change I made
to shortlog was unnecessary (shortlog already has a check to see if the
trailer key is empty) which makes the "is_trailer" check I added to it
here redundant (because non-trailer lines, which the new iterator can
iterate over, have empty keys).

Will remove the shortlog change in v4. Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux