Re: Lessons from commit fest

From: Chris Browne <cbbrowne(at)acm(dot)org>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Lessons from commit fest
Date: 2008-04-16 18:22:15
Message-ID: 6063uhu0bs.fsf@dba2.int.libertyrms.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

bruce(at)momjian(dot)us (Bruce Momjian) writes:
> Chris Browne wrote:
>> bruce(at)momjian(dot)us (Bruce Momjian) writes:
>> > Magnus Hagander wrote:
>> >> > And I think adopting surrounding naming, commeting, coding conventions
>> >> > should come naturally as it can aide in copy-pasting too :)
>> >>
>> >> I think pg_indent has to be made a lot more portable and easy to use
>> >> before that can happen :-) I've run it once or twice on linux machines,
>> >> and it comes out with huge changes compared to what Bruce gets on his
>> >> machine. Other times, it doesn't :-) So yeah, it could be that it just
>> >> needs to be made easier to use, because I may certainly have done
>> >> something wrong.
>> >
>> > Agreed, pgindent is too cumbersome to require patch submitters to use.
>> > One idea would be to allow C files to be emailed and the indented
>> > version automatically returned via email.
>>
>> Would it be a terrible idea to...
>>
>> - Draw the indent code from NetBSD into src/tools/pgindent
>> - Build it _in place_ inside the code tree (e.g. - don't assume
>> it will get installed in /usr/local/bin)
>> - Thus have the ability to run it in place?
>
> Yes, but it bloats our code and people still need to generate the
> typedefs and follow the instructions. The other problem is if they run
> it on a file they have modified, it is going to adjust places they
> didn't touch, thereby making the patch harder to review.

The bloat is 154K, on a project with something around 260MB of code.
I don't think this is a particlarly material degree of bloat.

If it is included in src/tools/pgindent, you can add in a Makefile
such that it is automatically built, so the cost of running it goes
way down, so that it could be run all the time rather than once in a
great long while.

If it was being run *all* the time, would we not expect to find that
we would be seeing relatively smaller sets of changes coming out of
it?

We are presently at the extreme position where pgindent is run once in
a very long time (~ once a year), at pretty considerable cost, and
with the associated cost that a whole lot of indentation problems are
managed by hand.

If we ran pgindent really frequently, there would admittedly be the
difference that there would be a lot of little cases of
changes-from-pgindent being committed along the way, but [1] might it
not be cheaper to accept that cost, with the concomittant benefit that
you could tell patchers "Hey, run pgindent before submitting that
patch, and that'll clean up a number of of the issues." Yes, it
doesn't address code changes like typedef generation, but that never
was an argument against running pgindent...

[1] In cases where the differences primarily fall from differences in
indentation, "cvs diff -u" can drop out those differences when reading
the effects of a patch...
--
let name="cbbrowne" and tld="cbbrowne.com" in name ^ "@" ^ tld;;
http://linuxdatabases.info/info/sap.html
"A study in the Washington Post says that women have better verbal
skills than men. I just want to say to the authors of that study:
Duh." -- Conan O' Brien

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2008-04-16 18:22:51 Re: How to submit a patch
Previous Message Joshua D. Drake 2008-04-16 18:21:51 Re: Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout