On Wed, Feb 17, 2016 at 2:39 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Tue, Feb 16, 2016 at 3:49 PM, Jeff King <peff@xxxxxxxx> wrote: >> On Tue, Feb 16, 2016 at 03:12:29PM -0500, Eric Sunshine wrote: >>> > Did you consider just using string_list_split for this? AFAICT, you >>> > don't care about the results being strbufs themselves, and it would do >>> > what you want without having to bother with patch 1. [...] >>> >>> That's a nice idea, however, I'm not sure if making it part of this >>> series this late in the game is a good idea. The series has gone >>> through major changes and heavy review in each of the preceding >>> versions, and turnaround time has been consequently quite slow (due >>> both to the amount of work required by Karthik for each version, and >>> to the amount of time needed by reviewers to digest all the new >>> changes). v4 was the first one which had settled to the point where >>> only minor changes were needed, and we were hoping to land the series >>> with v5. [...] >>> >>> With that in mind, it might be better to make this change as a >>> followup to this series. On the other hand, as you say, waiting would >>> expand the strbuf_split interface undesirably, so the alternative >>> would be for Karthik to submit v6 with this change only (to wit: drop >>> patch 1 and rewrite patch 2 as you've shown). While such a change will >>> again require careful review, at least it is well localized, and >>> Karthik's turnaround time shouldn't be too bad. So... >> >> Yeah, I don't insist, and like I said, I'm not 100% sure we can get rid >> of the strbuf_split interface anyway. I thought it might actually make >> things easier by making the series _shorter_ (so my regret was that >> mentioning earlier could have saved reviewing effort on patch 1). >> >> It does mean extra review of the patch I posted, but my hope was that >> it's small and localized, and wouldn't impact the later stuff seriously >> (there are some textual tweaks to carry it forward, though). > > My initial reaction was negative due to the heavy review burden this > series has demanded thus far, however, my mind was changing even as I > composed the above response. In retrospect, I think I'd be okay seeing > a v6, for the following reasons: > > - I already ended up reviewing the the suggested new changes pretty > closely as a side-effect of reading your proposal. > > - It would indeed be nice to avoid introducing > strbuf_split_str_omit_term() in the first place; thus one less thing > to worry about if someone ever takes on the task of retiring the > strbuf_split interface. > > - It should be only a minimal amount of work for Karthik, thus > turnaround time should be short. > > So, I think I'm fine with it, if Karthik is game. Sounds good to me. I just read the conversation between Jeff, Junio and You about the whitespace counter-argument and I think its good to go ahead with v6 with Jeff's suggested change. Since he's already pushed the changes on top of my changes to: git://github.com/peff/git.git jk/tweaked-ref-filter I'll just have a look and push that to the list as v6. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html