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

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.

Marko Tiikkaja

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to