Subject: Re: [PATCH 1/1] merge-recursive: avoid directory rename detection in recursive case

From: Emily Shaffer <[email-masked]>
Date: Tue, 6 Aug 2019 14:54:46 -0700

> > > +
> > > +             test_i18ngrep ! CONFLICT out &&
> > > +             test_i18ngrep ! BUG: err &&
> >
> > The BUG is gone. But should it not use i18ngrep? BUG() isn't localized.
> 
> Technically, yes, you're right. However, this line's purpose isn't
> correctness of the test but documentation for the person reading the
> testcase about what it's real original purpose was; my real test was
> the "test_must_be_empty err" check I have below it, but I added this
> line just to document the intent better.  I kind of like the
> "CONFLICT" and "BUG" lines looking similar just so the reader of the
> testcase doesn't have to try to reason through why they are different,
> although I guess it does present the problem that more careful readers
> like yourself might do a double take.
> 
> If folks find it more readable to use regular grep instead of
> test_i18ngrep, I can change this line as well as the "core dumped"
> check immediately below over to regular grep.
> 

It's fine by me, I see your point.

> Thanks for taking a look.  :-)

Reviewed-by: Emily Shaffer <[email-masked]>