From: Ben Peart Date: Fri, 27 Oct 2017 03:36:13 GMT Subject: Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman Message-ID: <11f3e539-1a4f-0572-a712-9f1c50359389@gmail.com> In-Reply-To: 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.