On Fri, Jan 18, 2019 at 3:10 AM Johannes Schindelin
[off-list ref] wrote:

Hi Duy,

the change itself looks good, but...

On Thu, 17 Jan 2019, Nguyễn Thái Ngọc Duy wrote:

quoted

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