From: Robert Haas on
On Sun, Jan 3, 2010 at 10:17 PM, Takahiro Itagaki
<itagaki.takahiro(a)oss.ntt.co.jp> wrote:
> Robert Haas <robertmhaas(a)gmail.com> wrote:
>
>> I have reviewed this patch and I think it looks pretty good.  A couple
>> of minor nits:
>>
>> - There are needless whitespace changes in the definition of struct
>> Counters.  The changes to the existing four members should be
>> reverted, and the new members should be made to match the existing
>> members.
>
> That's because the 'shared_blks_written' field is too long to keep the
> existing indentations. Since we still have some rooms in 80 columns,
> I'd like to change all of them as the previous patch.

I don't necessarily know what the right thing to do with the new ones
is, but I am pretty sure that pg_indent will revert any changes you
make to the existing ones. Assuming that's so, it doesn't make sense
to change them.

....Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on
Robert Haas <robertmhaas(a)gmail.com> writes:
> On Sun, Jan 3, 2010 at 10:17 PM, Takahiro Itagaki
> <itagaki.takahiro(a)oss.ntt.co.jp> wrote:
>> Robert Haas <robertmhaas(a)gmail.com> wrote:
>>> - There are needless whitespace changes in the definition of struct
>>> Counters. �The changes to the existing four members should be
>>> reverted, and the new members should be made to match the existing
>>> members.
>>
>> That's because the 'shared_blks_written' field is too long to keep the
>> existing indentations. Since we still have some rooms in 80 columns,
>> I'd like to change all of them as the previous patch.

> I don't necessarily know what the right thing to do with the new ones
> is, but I am pretty sure that pg_indent will revert any changes you
> make to the existing ones.

That it will. The proposed changes to the existing lines are an
exercise in uselessness; and to the extent that you format the added
lines with this layout in mind, the final result could be worse than
what you'd get if you adapt to pg_indent's rules to start with.

One possibility is to adopt shorter field names than these.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Takahiro Itagaki on

Tom Lane <tgl(a)sss.pgh.pa.us> wrote:

> > I don't necessarily know what the right thing to do with the new ones
> > is, but I am pretty sure that pg_indent will revert any changes you
> > make to the existing ones.
>
> That it will. The proposed changes to the existing lines are an
> exercise in uselessness; and to the extent that you format the added
> lines with this layout in mind, the final result could be worse than
> what you'd get if you adapt to pg_indent's rules to start with.

Here is the proposed patch to adjust white spaces.
It does not indent variables, but indents comments of the variables
to adjust other fields. Are those changes ok?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

From: Robert Haas on
On Thu, Jan 7, 2010 at 3:31 AM, Takahiro Itagaki
<itagaki.takahiro(a)oss.ntt.co.jp> wrote:
>
> Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
>
>> > I don't necessarily know what the right thing to do with the new ones
>> > is, but I am pretty sure that pg_indent will revert any changes you
>> > make to the existing ones.
>>
>> That it will.  The proposed changes to the existing lines are an
>> exercise in uselessness; and to the extent that you format the added
>> lines with this layout in mind, the final result could be worse than
>> what you'd get if you adapt to pg_indent's rules to start with.
>
> Here is the proposed patch to adjust white spaces.
> It does not indent variables, but indents comments of the variables
> to adjust other fields. Are those changes ok?

I think so. I'm not sure if it will push out the comment that is
immediately adjacent to the trailing semicolon, but I don't think it
will decrease the indent on the ones you've indented more. I think
this is close enough for now and you should go ahead and commit it.

....Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Takahiro Itagaki on

Robert Haas <robertmhaas(a)gmail.com> wrote:

> I think so. I'm not sure if it will push out the comment that is
> immediately adjacent to the trailing semicolon, but I don't think it
> will decrease the indent on the ones you've indented more. I think
> this is close enough for now and you should go ahead and commit it.

Thanks for your review. I committed it.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers