Hi, On Sat, Aug 29, 2020 at 11:58 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Hariom verma <hariom18599@xxxxxxxxx> writes: > > > On Fri, Aug 28, 2020 at 3:14 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > >> > >> [Cooking] > >> > >> * hv/ref-filter-trailers-atom-parsing-fix (2020-08-21) 2 commits > >> (merged to 'next' on 2020-08-24 at 79b27f3263) > >> + ref-filter: 'contents:trailers' show error if `:` is missing > >> + t6300: unify %(trailers) and %(contents:trailers) tests > >> ... > > > > After a discussion, we agreed on keeping the helper function instead > > on duplicating code in "ref-filter: 'contents:trailers' show error if > > `:` is missing" > > There is a high possibility we might want to reuse helper in other places too. > > Especially in the case of newly added "%(subject:sanitize)", if we > > also want "%(contents:subject:sanitize)" to work. > > > > Full discussion: > > https://public-inbox.org/git/CA+CkUQ8Gst2RTaXY6t+ytWu_9Pu7eqnRYRrnawRwYd_NN=u0Lg@xxxxxxxxxxxxxx/ > > > > I'm about to send the updated patch series soon. > > IIRC, the code in question is good for the purpose of what already > exists and is easier to follow without helper. When we need to make > the code more elaborabed and/or when we actually have the second > callsite would be the ideal time to introduce such a helper as a > preliminay clean-up patch early in such a follow-on series that > would happen after the "fix" in question graduates, I would think. > > To be honest, I am not sure if we even need an incremental on top > right now, unless we want to delay the two patches to fix real > breakage above and make them wait for patches that adds features > that require to call the same helper from elsewhere. > Yeah, I agree with you. Let's not delay these 2 patches. Sorry for the noise though. Thanks, Hariom
