On Mon, 2009-12-14 at 06:51 -0500, Robert Haas wrote:
> On Mon, Dec 14, 2009 at 6:35 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > On Mon, 2009-12-14 at 06:21 -0500, Robert Haas wrote:
> >> On Mon, Dec 14, 2009 at 6:11 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> >> > On Mon, 2009-12-14 at 11:09 +0100, Magnus Hagander wrote:
> >> >> On Mon, Dec 14, 2009 at 10:54, Heikki Linnakangas
> >> >> <heikki.linnakangas@enterprisedb.com> wrote:
> >> >> > * Please remove any spurious whitespace. "git diff --color" makes them
> >> >> > stand out like a sore thumb, in red. (pgindent will fix them but always
> >> >> > better to fix them before committing, IMO).
> >> >>
> >> >> +1 in general, not particularly for this patch (haven't checked that
> >> >> in this patch).
> >> >>
> >> >> Actually, how about we add that to the page at
> >> >> http://wiki.postgresql.org/wiki/Submitting_a_Patch?
> >> >
> >> > If we can define "spurious whitespace" it would help decide whether
> >> > there is any action to take, and when.
> >>
> >> git defines it as either (1) extra whitespace at the end of a line or
> >> (2) an initial indent that uses spaces followed by tabs (typically
> >> something like space-tab, where tab alone would have produced the same
> >> result). git diff --check master tends to be useful here.
> >
> > (2) is a problem that has been discussed before on hackers, anything
> > like that should be changed.
> >
> > Why is (1) important, and if it is important, why is it being mentioned
> > only now? Are we saying that all previous reviewers of my work (and
> > others') removed these without ever mentioning they had done so?
>
> pgident will remove such white spaces and create merge conflicts for
> everyone working on those areas of the code. I certainly mention this
> in any review I do where it's applicable, and have been doing so for
> some time. I also will certainly fix it for any code I commit. I
> also mentioned it in the review that I did of Hot Standby.
I don't recall you mentioning that.
There are no changes in this patch that are purely whitespace changes.
Those were removed prior to patch submission. This is all about code I
am adding or changing. My additions may disrupt their patches, but not
because of the whitespace.
If we are going to run pgindent anyway, what is the point? Seems like we
need a tool to fix patches, not a tool to fix the code.
I've made some changes to the larger files.
-- Simon Riggs www.2ndQuadrant.com
