From: Robert Haas on 3 Jan 2010 22:19 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 3 Jan 2010 23:12 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 7 Jan 2010 03:31 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 7 Jan 2010 10:33 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 7 Jan 2010 19:52 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
First
|
Prev
|
Pages: 1 2 Prev: PATCH: Add hstore_to_json() Next: Time to run initdb is mostly figure-out-the-timezone work |