Thread: 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@agliodbs.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@gmail.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@agliodbs.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@pgadmin.org> wrote: > On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh@agliodbs.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@gmail.com> wrote: > On Jul 2, 2009, at 3:41 AM, Dave Page <dpage@pgadmin.org> wrote: > >> On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh@agliodbs.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@pgadmin.org> wrote: > On Thu, Jul 2, 2009 at 12:04 PM, Robert Haas<robertmhaas@gmail.com> wrote: >> On Jul 2, 2009, at 3:41 AM, Dave Page <dpage@pgadmin.org> wrote: >> >>> On Thu, Jul 2, 2009 at 1:21 AM, Josh Berkus<josh@agliodbs.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@gmail.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@guedesoft.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@gmail.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@gmail.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@guedesoft.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@gmail.com> wrote: > On Thu, Jul 2, 2009 at 7:11 AM, Dave Page<dpage@pgadmin.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@guedesoft.net> wrote: > Em Thu, 02 Jul 2009 00:15:22 -0300, Robert Haas <robertmhaas@gmail.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@gmail.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@gmail.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@gmail.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@mit.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.Kotala@sun.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.Kotala@sun.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.Kotala@sun.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.Kotala@sun.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@guedesoft.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.Kotala@sun.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.Kotala@sun.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@example.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@example.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@momjian.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@example.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@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.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@example.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@momjian.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@gmail.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
Attachment
On Wed, Jul 8, 2009 at 1:11 AM, Joshua Tolley<eggyknap@gmail.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@gmail.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