Re: First CommitFest: July 15th

Folks,

There's been a lot of discussion/argument around how to handle the last
commitfest, but there seems to be a total consensus that we want to have
the first CF on July 15th.

I'd like Robert Haas to be the CF Manager for that commitfest if he's
available. I can help by running RRR or something.

Robert, I have not reviewed your software. Do I need a login or something?

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

On Wed, Jul 1, 2009 at 8:21 PM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:
> There's been a lot of discussion/argument around how to handle the last
> commitfest, but there seems to be a total consensus that we want to have the
> first CF on July 15th.
>
> I'd like Robert Haas to be the CF Manager for that commitfest if he's
> available.  I can help by running RRR or something.

Sounds great. How do we go about putting together a list of available
reviewers? Should we create a wikitable to which people can add
themselves?

> Robert, I have not reviewed your software.  Do I need a login or something?

Just your community login @ http://coridan.postgresql.org/ - but if
you send me your community login name I'll give you admin privs, which
will allow you to create/modify/delete commitfests and delete comments
that you yourself did not create.

The main problem is that my fine software only contains test data at
this point. If we have any volunteers who are available to migrate
the information from the Wiki to my app (which will involve a fair
amount of legwork), then I recommend that we accept their help as it
will make things easier for the CommitFest management team... which I
am all in favor of, especially now that I'm on that team. On the
other hand, if we don't have any volunteers, then I recommend that we
continue to use the Wiki for this CommitFest but make sure that all
patches for the next CommitFest, and any that follow, get added via
the app.

So, any volunteers?

Thanks,

...Robert

2009/7/2 Robert Haas <robertmhaas(at)gmail(dot)com>:
> The main problem is that my fine software only contains test data at
> this point.  If we have any volunteers who are available to migrate
> the information from the Wiki to my app (which will involve a fair
> amount of legwork),

As the original author of the CF wiki templates I feel like this
problem belongs to me. One of my primary goals was getting some
structure into the commitfests so that we could in fact export them to
a database at some point in the future. I'm glad that time has
finally come!

I imagine that "migration" would basically be converting the wiki data
into SQL, so I would need the database schema underlying the new CF
app.

Cheers,
BJ

On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:

> Robert, I have not reviewed your software.  Do I need a login or something?

It's not in productions yet as It's not been documented or given a
public hostname/servicename, both of which are requirements for any
postgresql.org services. As far as I'm aware, there's been no code
review yet either, which would probably be a good idea.

Which reminds me - that pentabarf installation is getting dangerously
close to being shut down...

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

On Jul 2, 2009, at 3:41 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:
>
>> Robert, I have not reviewed your software. Do I need a login or
>> something?
>
> It's not in productions yet as It's not been documented or given a
> public hostname/servicename, both of which are requirements for any
> postgresql.org services. As far as I'm aware, there's been no code
> review yet either, which would probably be a good idea.
>
> Which reminds me - that pentabarf installation is getting dangerously
> close to being shut down...

How do you recommend that we try to address these issues?

...Robert

On Thu, Jul 2, 2009 at 12:04 PM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
> On Jul 2, 2009, at 3:41 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:
>>
>>> Robert, I have not reviewed your software.  Do I need a login or
>>> something?
>>
>> It's not in productions yet as It's not been documented or given a
>> public hostname/servicename, both of which are requirements for any
>> postgresql.org services. As far as I'm aware, there's been no code
>> review yet either, which would probably be a good idea.
>>
>> Which reminds me - that pentabarf installation is getting dangerously
>> close to being shut down...
>
> How do you recommend that we try to address these issues?

Suggest a suitable name by which we can address the service (we don't
use the internal names like coridan because things can get moved
around), and I can set that up in a few minutes.

The documentation is the important bit as we don't deploy any service
without proper documentation any more (see pgFoundry for reasons why
not). We don't have a fixed format for that - what we need is a
description of what software is installed, how it's configured, and so
on. Enough that any of the sysadmin team can figure out in a couple of
minutes where the database is and how to access it, or what webserver
is being used and how to restart it etc. It should be enough that in a
pinch we can rebuild the server without lots of head-scratching.

We also need a list of key config files, which will be added to our
autobackup system to ensure we have copies and some change tracking.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

On Thu, Jul 2, 2009 at 7:11 AM, Dave Page<dpage(at)pgadmin(dot)org> wrote:
> On Thu, Jul 2, 2009 at 12:04 PM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
>> On Jul 2, 2009, at 3:41 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh(at)agliodbs(dot)com> wrote:
>>>
>>>> Robert, I have not reviewed your software.  Do I need a login or
>>>> something?
>>>
>>> It's not in productions yet as It's not been documented or given a
>>> public hostname/servicename, both of which are requirements for any
>>> postgresql.org services. As far as I'm aware, there's been no code
>>> review yet either, which would probably be a good idea.
>>>
>>> Which reminds me - that pentabarf installation is getting dangerously
>>> close to being shut down...
>>
>> How do you recommend that we try to address these issues?
>
> Suggest a suitable name by which we can address the service (we don't
> use the internal names like coridan because things can get moved
> around), and I can set that up in a few minutes.

pgcommitfest? or just commitfest?

> The documentation is the important bit as we don't deploy any service
> without proper documentation any more (see pgFoundry for reasons why
> not). We don't have a fixed format for that - what we need is a
> description of what software is installed, how it's configured, and so
> on. Enough that any of the sysadmin team can figure out in a couple of
> minutes where the database is and how to access it, or what webserver
> is being used and how to restart it etc. It should be enough that in a
> pinch we can rebuild the server without lots of head-scratching.

OK. Unfortunately, it's been a while said I did it, but it was mostly
a matter of installing the right set of ports, mostly Perl packages
like Template and Date::Calc. I will try to write something up.

> We also need a list of key config files, which will be added to our
> autobackup system to ensure we have copies and some change tracking.

OK. That should be easy to document.

...Robert

Em Thu, 02 Jul 2009 00:26:14 -0300, Brendan Jurd <direvus(at)gmail(dot)com>
escreveu:
> I imagine that "migration" would basically be converting the wiki data
> into SQL, so I would need the database schema underlying the new CF
> app.

How about parsing wiki content and create a "migration script" based on
pgcommitfest tables sctructure [1]?

[1]
http://git.postgresql.org/gitweb?p=pgcommitfest.git;a=blob;f=etc/table.sql;h=c60a298c863ef3e88dcfd16572781d2b435ca629;hb=HEAD

--
Dickson S. Guedes
mail/xmpp: guedes(at)guedesoft(dot)net - skype: guediz
http://guedesoft.net - http://www.postgresql.org.br

http://www.rnp.br/keyserver/pks/lookup?search=0x8F3E3C06D428D10A

On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
> As far as I'm aware, there's been no code
> review yet either, which would probably be a good idea.

I don't have loads of time in the coming days, but IIRC I've taken a glance at
a past version of the code, and would be willing to do so again, if it would
be useful.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
> On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
>> As far as I'm aware, there's been no code
>> review yet either, which would probably be a good idea.
>
> I don't have loads of time in the coming days, but IIRC I've taken a glance at
> a past version of the code, and would be willing to do so again, if it would
> be useful.

If you can look over it, that would be great. i'm not really qualified
to review perl code, and we always prefer to have at least two sets of
eyeballs on anything that we put into production for obvious reasons.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Em Thu, 02 Jul 2009 00:15:22 -0300, Robert Haas <robertmhaas(at)gmail(dot)com>
escreveu:
> The main problem is that my fine software only contains test data at
> this point. If we have any volunteers who are available to migrate
> the information from the Wiki to my app (which will involve a fair
> amount of legwork), then I recommend that we accept their help as it
> will make things easier for the CommitFest management team... which I
> am all in favor of, especially now that I'm on that team. On the
> other hand, if we don't have any volunteers, then I recommend that we
> continue to use the Wiki for this CommitFest but make sure that all
> patches for the next CommitFest, and any that follow, get added via
> the app.
>
> So, any volunteers?

I don't know if this tool will be approved yet, but I'm working on
copying the Wiki entries of CommitFest to pgcommitfest.

Until now i created the following CommitFest Topics based on topics in
the Wiki:

* Contrib modules
* EXPLAIN
* Error Reporting
* Instrumentation
* Miscellaneous
* My New Topic
* Performance
* Replication
* SQL language features
* Security

All the patches on Miscellaneous topic in Wiki was copied to coridan, but i
couldn't copy comments of patches thath have one.

Would be nice if a theres is a way to order by some column like Patch Name,
Topic, Status, Author and Last Activity for example. Some descriptions was
truncated because de maxsize of textbox.

regards,
--
Dickson S. Guedes
mail/xmpp: guedes(at)guedesoft(dot)net - skype: guediz
http://guedesoft.net - http://www.postgresql.org.br

http://www.rnp.br/keyserver/pks/lookup?search=0x8F3E3C06D428D10A

On Thu, Jul 2, 2009 at 12:48 PM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Jul 2, 2009 at 7:11 AM, Dave Page<dpage(at)pgadmin(dot)org> wrote:
>> Suggest a suitable name by which we can address the service (we don't
>> use the internal names like coridan because things can get moved
>> around), and I can set that up in a few minutes.
>
> pgcommitfest?  or just commitfest?

> commitfest.postgresql.org
Server: mx3.hub.org
Address: 206.223.169.73#53

commitfest.postgresql.org canonical name = coridan.postgresql.org.
Name: coridan.postgresql.org
Address: 98.129.198.114

> OK.  Unfortunately, it's been a while said I did it, but it was mostly
> a matter of installing the right set of ports, mostly Perl packages
> like Template and Date::Calc.  I will try to write something up.

Thanks.

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

On Thu, Jul 2, 2009 at 11:05 AM, Dickson S. Guedes<listas(at)guedesoft(dot)net> wrote:
> Em Thu, 02 Jul 2009 00:15:22 -0300, Robert Haas <robertmhaas(at)gmail(dot)com>
> escreveu:
>>
>> The main problem is that my fine software only contains test data at
>> this point.  If we have any volunteers who are available to migrate
>> the information from the Wiki to my app (which will involve a fair
>> amount of legwork), then I recommend that we accept their help as it
>> will make things easier for the CommitFest management team...  which I
>> am all in favor of, especially now that I'm on that team.  On the
>> other hand, if we don't have any volunteers, then I recommend that we
>> continue to use the Wiki for this CommitFest but make sure that all
>> patches for the next CommitFest, and any that follow, get added via
>> the app.
>>
>> So, any volunteers?
>
> I don't know if this tool will be approved yet, but I'm working on
> copying the Wiki entries of CommitFest to pgcommitfest.
>
> Until now i created the following CommitFest Topics based on topics in
> the Wiki:
>
> * Contrib modules
> * EXPLAIN
> * Error Reporting
> * Instrumentation
> * Miscellaneous
> * My New Topic
> * Performance
> * Replication
> * SQL language features
> * Security
>
> All the patches on Miscellaneous topic in Wiki was copied to coridan, but i
> couldn't copy comments of patches thath have one.
>
> Would be nice if a theres is a way to order by some column like Patch Name,
> Topic, Status, Author and Last Activity for example. Some descriptions was
> truncated because de maxsize of textbox.

Brendan Jurd I think is working on an awk script - you probably want
to coordinate with him...

...Robert

On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
> On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
> > On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
> >> As far as I'm aware, there's been no code
> >> review yet either, which would probably be a good idea.
> >
> > I don't have loads of time in the coming days, but IIRC I've taken a glance at
> > a past version of the code, and would be willing to do so again, if it would
> > be useful.
>
> If you can look over it, that would be great. i'm not really qualified
> to review perl code, and we always prefer to have at least two sets of
> eyeballs on anything that we put into production for obvious reasons.

Is git://git.postgresql.org/git/pgcommitfest.git still the right place to get
the source?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

On Thu, Jul 2, 2009 at 1:10 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
> On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
>> On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
>> > On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
>> >> As far as I'm aware, there's been no code
>> >> review yet either, which would probably be a good idea.
>> >
>> > I don't have loads of time in the coming days, but IIRC I've taken a glance at
>> > a past version of the code, and would be willing to do so again, if it would
>> > be useful.
>>
>> If you can look over it, that would be great. i'm not really qualified
>> to review perl code, and we always prefer to have at least two sets of
>> eyeballs on anything that we put into production for obvious reasons.
>
> Is git://git.postgresql.org/git/pgcommitfest.git still the right place to get
> the source?

Yes.

...Robert

Josh Berkus píše v st 01. 07. 2009 v 17:21 -0700:
> Folks,
>
> There's been a lot of discussion/argument around how to handle the last
> commitfest, but there seems to be a total consensus that we want to have
> the first CF on July 15th.

Can we add flags like bump catalog version, bump page layout version,
modify AM for each patch? It should help to track pg_upgrade changes.

Zdenek

So I currently have some free time as I'm currently between jobs. I
can start going through the queued patches in the next few weeks.
Should I be looking at the wiki currently? Or is your tool ready to
go?

--
greg
http://mit.edu/~gsstark/resume.pdf

On Thu, Jul 2, 2009 at 5:50 PM, Greg Stark<gsstark(at)mit(dot)edu> wrote:
> So I currently have some free time as I'm currently between jobs. I
> can start going through the queued patches in the next few weeks.
> Should I be looking at the wiki currently? Or is your tool ready to
> go?

Wiki for now. Brendan Jurd is working on a script to bulk-import the
data to the tool, and he (and I) will make sure to update -hackers and
redirect the wiki page when he gets that done. I'm hoping that it
will happen in the next few days.

...Robert

On Thu, Jul 2, 2009 at 3:42 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>
> Josh Berkus píše v st 01. 07. 2009 v 17:21 -0700:
>> Folks,
>>
>> There's been a lot of discussion/argument around how to handle the last
>> commitfest, but there seems to be a total consensus that we want to have
>> the first CF on July 15th.
>
> Can we add flags like bump catalog version, bump page layout version,
> modify AM for each patch? It should help to track pg_upgrade changes.

That's not a bad idea, and it wouldn't be hard to add various flags
and things to the CommitFest app I wrote, but how would we maintain
the information and keep it correct? It seems like there might be a
danger that patch authors wouldn't know whether or not they were doing
those things. Also, how would we handle changes by committers, who
don't always go through the CommitFest process?

Not sure of the answers here, just thinking out loud.

...Robert

On Friday 03 July 2009 05:16:41 Robert Haas wrote:
> On Thu, Jul 2, 2009 at 3:42 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:
> > Josh Berkus píše v st 01. 07. 2009 v 17:21 -0700:
> >> Folks,
> >>
> >> There's been a lot of discussion/argument around how to handle the last
> >> commitfest, but there seems to be a total consensus that we want to have
> >> the first CF on July 15th.
> >
> > Can we add flags like bump catalog version, bump page layout version,
> > modify AM for each patch? It should help to track pg_upgrade changes.
>
> That's not a bad idea, and it wouldn't be hard to add various flags
> and things to the CommitFest app I wrote, but how would we maintain
> the information and keep it correct? It seems like there might be a
> danger that patch authors wouldn't know whether or not they were doing
> those things. Also, how would we handle changes by committers, who
> don't always go through the CommitFest process?

I think this information could be computed automatically, if someone cared
enough. It shouldn't be necessary to bother every single participant in the
process with this.

Peter Eisentraut píše v pá 03. 07. 2009 v 09:19 +0300:
> On Friday 03 July 2009 05:16:41 Robert Haas wrote:
> > On Thu, Jul 2, 2009 at 3:42 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:
> > > Josh Berkus píše v st 01. 07. 2009 v 17:21 -0700:
> > >> Folks,
> > >>
> > >> There's been a lot of discussion/argument around how to handle the last
> > >> commitfest, but there seems to be a total consensus that we want to have
> > >> the first CF on July 15th.
> > >
> > > Can we add flags like bump catalog version, bump page layout version,
> > > modify AM for each patch? It should help to track pg_upgrade changes.
> >
> > That's not a bad idea, and it wouldn't be hard to add various flags
> > and things to the CommitFest app I wrote, but how would we maintain
> > the information and keep it correct? It seems like there might be a
> > danger that patch authors wouldn't know whether or not they were doing
> > those things. Also, how would we handle changes by committers, who
> > don't always go through the CommitFest process?
>
> I think this information could be computed automatically, if someone cared
> enough. It shouldn't be necessary to bother every single participant in the
> process with this.

I think that developer is responsible for his patch. He should know what
he doing. When he will send a patch and see checkbox like "modified AM?"
then he should know what he modified? It is also warning for commiter
that catalog version has to be bumped during a commit.

I don't see any method how to check automatically. Something could be
possible - like structure checker. But when meaning of data is going to
be different.

Zdenek

Robert Haas píše v čt 02. 07. 2009 v 22:16 -0400:
> On Thu, Jul 2, 2009 at 3:42 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:

> Also, how would we handle changes by committers, who
> don't always go through the CommitFest process?

I think that all head patches should go to through a new tool for
recording also in case when developer is commiter itself.

Zdenek

"Dickson S. Guedes" <listas(at)guedesoft(dot)net> wrote:

> pgcommitfest tables sctructure [1]?
>
> [1]
>
http://git.postgresql.org/gitweb?p=pgcommitfest.git;a=blob;f=etc/table.sql;h=c60a298c863ef3e88dcfd16572781d2b435ca629;hb=HEAD

On minor quibble with this schema: I believe that session.login_time
should be TIMESTAMP WITH TIME ZONE.

-Kevin

On Fri, Jul 3, 2009 at 3:22 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:
> Robert Haas píše v čt 02. 07. 2009 v 22:16 -0400:
>> On Thu, Jul 2, 2009 at 3:42 PM, Zdenek Kotala<Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>
>> Also, how would we handle changes by committers, who
>> don't always go through the CommitFest process?
>
> I think that all head patches should go to through a new tool for
> recording also in case when developer is commiter itself.

You'll have to put that argument to the committers; but I expect a
cool reception. I think what would be more useful is if we could
somehow associated metadata with each commit. Right now, for example,
the author of a patch is not stored with the patch in any structured
way; it's just typed in, usually but not always as the last line of
the commit. So you can't easily find out what lines of code a certain
person has touched, for example. The sorts of problems that you're
talking about seem broadly in the same vein.

...Robert

Josh Berkus wrote:
> Folks,...the first CF on July 15th.

Would it make the CommitFest easier if there were an additional
column which indicates what CVS-version of Postgres the patch
cleanly applies to?

Perhaps a patch submitter could indicate the CVS date/time
with which he developed the patch. If a reviewer happens
to apply the patch on a later version he could update it as
cleanly applying at that later date.

Commiters could feel free to ignore patches that are
sufficiently far off of HEAD, so it might make work easier
for them too.

Ron Mayer wrote:
> Josh Berkus wrote:
>> Folks,...the first CF on July 15th.
>
> Would it make the CommitFest easier if there were an additional
> column which indicates what CVS-version of Postgres the patch
> cleanly applies to?
>
> Perhaps a patch submitter could indicate the CVS date/time
> with which he developed the patch. If a reviewer happens
> to apply the patch on a later version he could update it as
> cleanly applying at that later date.
>
> Commiters could feel free to ignore patches that are
> sufficiently far off of HEAD, so it might make work easier
> for them too.
>
>

I think the patch should apply cleanly to HEAD at the time it is
submitted. The actual CVS versions should be visible in the patch.
Normally we will try to take care of any subsequent bitrot - I don't
think developers should have to pay too high a price for our processes.
Occasionally a developer will be asked to help in removing the bitrot,
but that is usually the first thing I try to do in a review, after
applying the simple eyeballs test.

In theory this is an area where a more sophisticated SCM system will
help us some.

(Actually, you often learn a lot that way - it's a good exercise for all
of us.)

cheers

andrew

On Saturday 04 July 2009 00:54:11 Robert Haas wrote:
> I think what would be more useful is if we could
> somehow associated metadata with each commit. Right now, for example,
> the author of a patch is not stored with the patch in any structured
> way; it's just typed in, usually but not always as the last line of
> the commit. So you can't easily find out what lines of code a certain
> person has touched, for example. The sorts of problems that you're
> talking about seem broadly in the same vein.

I have been trying to follow a convention on-and-off to put the author of the
patch in the last line of the commit message, like

Author: First Last <name(at)example(dot)com>

A tool such as git-cvsimport will actually parse that and put it into the
author field of a git commit. (The tool we use, fromcvs, doesn't do that, but
it could conceivably be patched easily to do it.)

I also found the following resource helpful in crafting commit messages:
http://www.tpope.net/node/106

Peter Eisentraut wrote:
> On Saturday 04 July 2009 00:54:11 Robert Haas wrote:
> > I think what would be more useful is if we could
> > somehow associated metadata with each commit. Right now, for example,
> > the author of a patch is not stored with the patch in any structured
> > way; it's just typed in, usually but not always as the last line of
> > the commit. So you can't easily find out what lines of code a certain
> > person has touched, for example. The sorts of problems that you're
> > talking about seem broadly in the same vein.
>
> I have been trying to follow a convention on-and-off to put the author of the
> patch in the last line of the commit message, like
>
> Author: First Last <name(at)example(dot)com>

Sure, I can use that format if we decide to be consistent.

> A tool such as git-cvsimport will actually parse that and put it into the
> author field of a git commit. (The tool we use, fromcvs, doesn't do that, but
> it could conceivably be patched easily to do it.)
>
> I also found the following resource helpful in crafting commit messages:
> http://www.tpope.net/node/106

Interesting idea to have a subject line for the commit message.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, Jul 06, 2009 at 09:12:55AM -0400, Bruce Momjian wrote:
> Peter Eisentraut wrote:
> > On Saturday 04 July 2009 00:54:11 Robert Haas wrote:
> > > I think what would be more useful is if we could
> > > somehow associated metadata with each commit. Right now, for example,
> > > the author of a patch is not stored with the patch in any structured
> > > way; it's just typed in, usually but not always as the last line of
> > > the commit. So you can't easily find out what lines of code a certain
> > > person has touched, for example. The sorts of problems that you're
> > > talking about seem broadly in the same vein.
> >
> > I have been trying to follow a convention on-and-off to put the author of the
> > patch in the last line of the commit message, like
> >
> > Author: First Last <name(at)example(dot)com>
>
> Sure, I can use that format if we decide to be consistent.
>
> > A tool such as git-cvsimport will actually parse that and put it into the
> > author field of a git commit. (The tool we use, fromcvs, doesn't do that, but
> > it could conceivably be patched easily to do it.)
> >
> > I also found the following resource helpful in crafting commit messages:
> > http://www.tpope.net/node/106
>
> Interesting idea to have a subject line for the commit message.

It would help me a lot when putting together the patches section in
the PostgreSQL Weekly News.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

David Fetter wrote:
> On Mon, Jul 06, 2009 at 09:12:55AM -0400, Bruce Momjian wrote:
> > Peter Eisentraut wrote:
> > > On Saturday 04 July 2009 00:54:11 Robert Haas wrote:
> > > > I think what would be more useful is if we could
> > > > somehow associated metadata with each commit. Right now, for example,
> > > > the author of a patch is not stored with the patch in any structured
> > > > way; it's just typed in, usually but not always as the last line of
> > > > the commit. So you can't easily find out what lines of code a certain
> > > > person has touched, for example. The sorts of problems that you're
> > > > talking about seem broadly in the same vein.
> > >
> > > I have been trying to follow a convention on-and-off to put the author of the
> > > patch in the last line of the commit message, like
> > >
> > > Author: First Last <name(at)example(dot)com>
> >
> > Sure, I can use that format if we decide to be consistent.
> >
> > > A tool such as git-cvsimport will actually parse that and put it into the
> > > author field of a git commit. (The tool we use, fromcvs, doesn't do that, but
> > > it could conceivably be patched easily to do it.)
> > >
> > > I also found the following resource helpful in crafting commit messages:
> > > http://www.tpope.net/node/106
> >
> > Interesting idea to have a subject line for the commit message.
>
> It would help me a lot when putting together the patches section in
> the PostgreSQL Weekly News.

OK, someone want to write a wiki that explains our new preferred commit
message format?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
> On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
> > On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
> >> As far as I'm aware, there's been no code
> >> review yet either, which would probably be a good idea.
> >
> > I don't have loads of time in the coming days, but IIRC I've taken a glance at
> > a past version of the code, and would be willing to do so again, if it would
> > be useful.
>
> If you can look over it, that would be great. i'm not really qualified
> to review perl code, and we always prefer to have at least two sets of
> eyeballs on anything that we put into production for obvious reasons.

On the assumption that other folks' testing has included bug hunting and the
like, I've spent the review time I was able to muster up figuring out the code
and looking for stuff that scared me. I didn't find anything that jumped out.
I did wonder if the %ACTION hash in Handler.pm ought not perhaps include a
flag to indicate that that action needs authentication, and have the handler
take care of that instead of the individual actions. Perhaps a similar
technique could be profitably employed for the titles. Oh, and in Patch.pm,
s/with/which in "patches with have been Committed".

Finally, I ran Perl::Critic, and attached an (admittedly untested) patch to
clean up the things it whined about.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

On Wed, Jul 8, 2009 at 1:11 AM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
> On Thu, Jul 02, 2009 at 03:42:56PM +0100, Dave Page wrote:
>> On Thu, Jul 2, 2009 at 3:22 PM, Joshua Tolley<eggyknap(at)gmail(dot)com> wrote:
>> > On Thu, Jul 02, 2009 at 08:41:27AM +0100, Dave Page wrote:
>> >> As far as I'm aware, there's been no code
>> >> review yet either, which would probably be a good idea.
>> >
>> > I don't have loads of time in the coming days, but IIRC I've taken a glance at
>> > a past version of the code, and would be willing to do so again, if it would
>> > be useful.
>>
>> If you can look over it, that would be great. i'm not really qualified
>> to review perl code, and we always prefer to have at least two sets of
>> eyeballs on anything that we put into production for obvious reasons.
>
> On the assumption that other folks' testing has included bug hunting and the
> like, I've spent the review time I was able to muster up figuring out the code
> and looking for stuff that scared me. I didn't find anything that jumped out.
> I did wonder if the %ACTION hash in Handler.pm ought not perhaps include a
> flag to indicate that that action needs authentication, and have the handler
> take care of that instead of the individual actions.

Possibly so. We may also find that it needs to be a bit more
fine-grained than that, as there are already three levels of access
(public, login required, administrator login required) and there could
theoretically be more in the future.

> Perhaps a similar
> technique could be profitably employed for the titles. Oh, and in Patch.pm,
> s/with/which in "patches with have been Committed".

Fixed, thanks.

> Finally, I ran Perl::Critic, and attached an (admittedly untested) patch to
> clean up the things it whined about.

As usual, I'm unimpressed by the whining emitted by Perl::Critic. I
can understand that if a function is really intended to return void
(but perl doesn't have that concept) then you probably ought to write
just "return" rather than "return undef". But if the function
sometimes returns a value and sometimes returns "undef", insisting
that the word "undef" not be spelled out explicitly seems pretty
silly.

The other changes have marginally more merit, though some of them
break with surrounding whitespace conventions.

...Robert