Re: new json funcs

On 1/18/14, 9:38 PM, Andrew Dunstan wrote:
> On 01/18/2014 12:34 PM, Marko Tiikkaja wrote:
>> Is it possible for you to generate a diff that doesn't have all these
>> unrelated changes in it (from a pgindent run, I presume)?  I just read
>> through three pagefuls and I didn't see any relevant changes, but I'm
>> not sure I want to keep doing that for the rest of the patch.
>>
>
> This seems to be quite overstated. The chunks in the version 3 patch
> that only contain pgindent effects are those at lines 751,771,866 and
> 1808 of json.c, by my reckoning. All the other changes are more than
> formatting.

Oh I see, there's a version 3 which improves things by a lot.  I just 
took the latest patch from the CF app and that was the v2 patch.  Now 
looking at it again, I see that it actually added new lines around 
json.c:68, which I believe proves my point that reviewing such a patch 
is hard.

> And undoing the pgindent changes and then individually applying all but
> those mentioned above would take me a lot of time.

v3 looks "ok".  I would have preferred a patch with no unrelated 
changes, but I can live with what we have there.

Something like the first three pagefuls of v2, however, would take *me* 
a lot of time, which I believe is not acceptable.  I don't care why a 
patch has lots of unrelated stuff in it, I'm not going to waste my time 
trying to figure out which parts are relevant and which aren't.  That's 
a lot of time wasted just to end up with a review possibly full of 
missed problems and misunderstood code.

But I'll continue with my review now that this has been sorted out.


Regards,
Marko Tiikkaja