Re: Lessons from commit fest - Mailing list pgsql-hackers

From Chris Browne
Subject Re: Lessons from commit fest
Date
Msg-id 6063uhu0bs.fsf@dba2.int.libertyrms.com
Whole thread Raw
In response to Re: Lessons from commit fest  (Chris Browne <cbbrowne@acm.org>)
Responses Re: Lessons from commit fest  (Bruce Momjian <bruce@momjian.us>)
Re: Lessons from commit fest  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
bruce@momjian.us (Bruce Momjian) writes:
> Chris Browne wrote:
>> bruce@momjian.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


pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: How to submit a patch
Next
From: "Brendan Jurd"
Date:
Subject: Re: How to submit a patch