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. Days after the topic got merged to 'next' is not a time to send any updated patch series. It should come in the form of incremental update on top of what is already there. 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. Thanks.
