Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

[Ben Peart <peartben@gmail.com>]

On 10/26/2017 5:29 PM, Alex Vandiver wrote:
> On Thu, 26 Oct 2017, Ben Peart wrote:
>> On 10/25/2017 9:31 PM, Alex Vandiver wrote:
>>> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
>>> index a3e30bf54..79f24325c 100755
>>> --- a/t/t7519/fsmonitor-watchman
>>> +++ b/t/t7519/fsmonitor-watchman
>>> @@ -50,7 +50,7 @@ launch_watchman();
>>>      sub launch_watchman {
>>>    -	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
>>> +	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')
>>
>> Since this is a test script performance isn't critical.  This version of the
>> integration script logs the response to a file in .git/watchman-response.json
>> and is much more human readable without the "--no-pretty."  As such, I'd leave
>> this one pretty.
> 
> This would be the first delta between the test file and the template
> file.  It seems quite important to me to attempt to ensure that we're
> testing the _same_ contents that we're suggesting users set up.  In
> fact, it makes more sense to me to just turn this into a symlink to the
> sample template.
>   - Alex
> 

If you look closer (actually diff the two files) you will see that the 
test version includes logging that the sample doesn't include.  I found 
the logging very helpful during testing of the feature and debugging 
Watchman issues but don't want end users to pay the performance penalty 
of the logging.

t/t7519/fsmonitor-watchman isn't actually used by any of the automated 
tests as they can't assume Watchman is available. It is just provided as 
a convenience to enable manual testing and debugging.