On Fri, Jan 18, 2019 at 3:10 AM Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > > Hi Duy, > > the change itself looks good, but... > > On Thu, 17 Jan 2019, Nguyễn Thái Ngọc Duy wrote: > > > diff_opt_parse() is a heavy hammer to just set diff filter. But it's > > the only way because of the diff_status_letters[] mapping. Add a new > > API to set diff filter and use it in git-am. diff_opt_parse()'s only > > remaining call site in revision.c will be gone soon and having it here > > ... "will be gone soon"? Does that mean that you mail-bomb another mega > patch series iteration once you did that, now sending 77 or 78 patches? That's another 75 patches. > I don't know about others, but I can only afford to spend a fraction of my > waking hours on reviews, and even back when Christian sent the built-in am > as a loooong patch series it was *already* a big problem. Thankfully he > seems to have decided to never do that again. > > It would probably make sense to break your 76-strong patch series down > into at least four separate patch series, they would still be as long as > my Azure Pipelines one (which is longer than I am actually comfortable > with, but in my case, it was necessary, while your patch series consists > of many, mostly independent patches that could even be wrapped into > individual patch series of 1 or 2). It's just way too much to review if > you present it in the current manner. Sorry somehow I forgot about breaking down the series. Part of the reason though was I wanted to show that we got somewhere in the end, it's not just random restructuring patches that may end up getting reverted. If there are lots of changes in this series, I'll resend in smaller series. For the revision.c conversion I'll make sure to send in small series. > Ciao, > Johannes > -- Duy
