Re: [HACKERS] commitfest management webapp
On Tue, 26 May 2009, Robert Haas wrote: I'm not totally keen on pulling the subject lines. I know that's what we've mostly been doing, but sometimes the subject line is something like "patch to improve the way that foo does bar", rather than "make bar use baz algorithm" or (even worse) "patch to add support for foo" rather than "foo". I wasn't suggesting you pull the subject line and use it as a key for anything, was just thinking it would be nice to display it, as a way to double-check it points to the right place. -- * Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] commitfest management webapp
On Tue, May 26, 2009 at 10:15 PM, Greg Smith wrote: > On Tue, 26 May 2009, Robert Haas wrote: >> I'm open to suggestions on how to improve this situation, though, because >> it's definitely not ideal, and precludes things that reasonable people might >> want to do, like "contact the guy who submitted this patch", "contact the >> authors of all patches waiting for review", and similar. > > Since you're taking the message-id where the patch was submitted at as an > input, couldn't you scrape this information out of the archives? You > probably want to do a bit of that regardless; having the program pull and > display the author and subject line of the archived message is a good sanity > check that you entered the message ID correctly. I think something like this might work. I had a suggestion previously of just checking that the message-IDs are even valid, which might be a good place to start, and then I could try to figure out how to extend it along these lines. I'm not totally keen on pulling the subject lines. I know that's what we've mostly been doing, but sometimes the subject line is something like "patch to improve the way that foo does bar", rather than "make bar use baz algorithm" or (even worse) "patch to add support for foo" rather than "foo". Also, I think we may also want to assign each patch a shortname that can be used as an argument to command-line tools. I'd really like to be able to do something like this: $ pgcf-patch foo ...and have foo.patch show up in $CWD. Even swankier would be to make this integrate with git somehow. >> I know that there are some of you reading this who may think that we >> should convert to reviewboard or patchwork or some other system. I can say >> that personally I'm unimpressed by those suggestions because they will >> almost certainly require process changes that this does not > > We used Reviewboard a fair amount here at Truviso for a while. Lately a > good chunk of that patch review has been happening more efficiently by > passing pointers to private git branches around instead. I think you're > right to focus on just the review workflow and not the patch review itself, > let people use whatever tools they're already comfortable with for that > part. Thanks and well said. > I just spent a few minutes poking around your code and that quickly was able > to see how things fit together, which is certainly not something I can say > about Reviewboard etc. The interface looks good and the code easy enough to > improve. The main concerns I'm left with after that review are with how to > properly test the security of the code. Some maturity there is one major > thing that more packages in larger use have going for them vs. rolling your > own in this sort of situation. Well, the nice thing about it is that it's not a ton of code, so visual inspection ought to buy you something. But I obviously can't and don't promise that it is free of bugs, security-related or otherwise. I was pretty dismayed when I realized that Template's "| html" filter did not think apostrophes needed to be quoted, which they obviously do if you're going to use them in a context like . Now that's the one I caught - question is - what did I miss? ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] commitfest management webapp
On Tue, 26 May 2009, Robert Haas wrote: I'm open to suggestions on how to improve this situation, though, because it's definitely not ideal, and precludes things that reasonable people might want to do, like "contact the guy who submitted this patch", "contact the authors of all patches waiting for review", and similar. Since you're taking the message-id where the patch was submitted at as an input, couldn't you scrape this information out of the archives? You probably want to do a bit of that regardless; having the program pull and display the author and subject line of the archived message is a good sanity check that you entered the message ID correctly. I know that there are some of you reading this who may think that we should convert to reviewboard or patchwork or some other system. I can say that personally I'm unimpressed by those suggestions because they will almost certainly require process changes that this does not We used Reviewboard a fair amount here at Truviso for a while. Lately a good chunk of that patch review has been happening more efficiently by passing pointers to private git branches around instead. I think you're right to focus on just the review workflow and not the patch review itself, let people use whatever tools they're already comfortable with for that part. I just spent a few minutes poking around your code and that quickly was able to see how things fit together, which is certainly not something I can say about Reviewboard etc. The interface looks good and the code easy enough to improve. The main concerns I'm left with after that review are with how to properly test the security of the code. Some maturity there is one major thing that more packages in larger use have going for them vs. rolling your own in this sort of situation. -- * Greg Smith gsm...@gregsmith.com http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] commitfest management webapp
Back in January, there was some discussion of creation a web application to make it easier to manage CommitFests. http://archives.postgresql.org/message-id/20090127134245.ga6...@alvh.no-ip.org This was further discussed at PGCon, and I now have a working version for folks to play with. With the help of Dave Page, I was able to relearn how to use the BSD ports collection and get it up here: http://coridan.postgresql.org/ Feedback is appreciated, especially from people involved in previous commitfests as committers, reviewers, patch authors, or commitfest managers. The source code, in Perl, is available here: http://git.postgresql.org/gitweb?p=pgcommitfest.git;a=summary A few things to note: 1. You won't see a link anywhere to create a new CommitFest or edit the name of an existing CommitFest. This is not because the functionality doesn't exist, but because your community login account is not enabled with administrator privileges for this application. For the same reason, you won't be able to edit or delete comments other than your own. If you would like to have these great powers, please send me a private email with your community account name and I will power you up. If we decide to use this as official project infrastructure, then you might get un-powered up unless I or one of the CommitFest managers have some idea who you are. :-) 2. There are many things that this application doesn't do. One particularly glaring thing that it doesn't do is allow you to move a patch from one CommitFest to another CommitFest. This is basically a bug that I intend to fix, but it didn't seem necessary to fix it before putting the thing out there for people to look at and comment on. At any rate, if the application doesn't do something that you wish it did, please feel free to let me know what that thing is, or provide a patch. I'm very interested in making this better (unless of course you all hate it, in which case my interest in improving it will likely decline precipitously). 3. The integration with the community login system is currently rather poor. The problem is that we can't count on patch submitters to have a community login, and even if they do we can't count on the person adding the patch to the system to know what it is. We could of course require patch submitters to have a community login and to add their patches themselves, but I'm not really that keen on raising the bar for submitting a patch even to that modest extent. I'm open to suggestions on how to improve this situation, though, because it's definitely not ideal, and precludes things that reasonable people might want to do, like "contact the guy who submitted this patch", "contact the authors of all patches waiting for review", and similar. 4. The intent of this system is really just to get the data that is currently on the CommitFest pages into a database where, I venture to say, structured data about the development cycle of a database product properly belongs. I expect it to be possible to use this tool to build additional infrastructure to facilitate patch review, like an automated test to see whether all the latest versions of all the open patches actually still apply. (If we have a human being sanity check them to make sure they don't contain malicious code, we could also test for "compiles" and "passes regression tests", which would rock.) This infrastructure does not currently exist, but having the data in a database makes it feasible to think about doing it; suggestions are welcome, as is code. I know that there are some of you reading this who may think that we should convert to reviewboard or patchwork or some other system. I can say that personally I'm unimpressed by those suggestions because they will almost certainly require process changes that this does not, process changes which I suspect we're unprepared to make. But there's nothing to prevent you from setting up and advocating your system in this space. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers