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.