[Launchpad-reviewers] [Merge] lp:~julian-edwards/maas/celery-settings into lp:maas

2012-05-21 Thread Julian Edwards
Julian Edwards has proposed merging lp:~julian-edwards/maas/celery-settings into lp:maas. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~julian-edwards/maas/celery-settings/+merge/106568 The build is currently broken

[Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad

2012-05-21 Thread Steve Kowalik
Steve Kowalik has proposed merging lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~stevenk/launchpad/populate-spph-pu/+merge/105601 When calling

[Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad

2012-05-21 Thread Steve Kowalik
The proposal to merge lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad has been updated. Description changed to: When calling newSourcePublication() from IPackageUpload.realiseUpload(), pass in the new packageupload parameter, so that we can grab the packageupload easily after the

[Launchpad-reviewers] [Merge] lp:~wgrant/launchpad/bugsummary-v2-app-no-fixed-upstream into lp:launchpad

2012-05-21 Thread noreply
The proposal to merge lp:~wgrant/launchpad/bugsummary-v2-app-no-fixed-upstream into lp:launchpad has been updated. Status: Needs review = Merged For more details, see: https://code.launchpad.net/~wgrant/launchpad/bugsummary-v2-app-no-fixed-upstream/+merge/106557 --

Re: [Launchpad-reviewers] [Merge] lp:~wallyworld/launchpad/distro-page-ajax-835020 into lp:launchpad

2012-05-21 Thread Abel Deuring
Review: Approve code looks good now. Thanks for the additional changes! -- https://code.launchpad.net/~wallyworld/launchpad/distro-page-ajax-835020/+merge/106321 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing

[Launchpad-reviewers] [Merge] lp:~jtv/maas/non-cobbler-power-types into lp:maas

2012-05-21 Thread Jeroen T. Vermeulen
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/non-cobbler-power-types into lp:maas. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~jtv/maas/non-cobbler-power-types/+merge/106606 I hope this will fix the build.

Re: [Launchpad-reviewers] [Merge] lp:~rvb/maas/pserv-power-fix into lp:maas

2012-05-21 Thread Raphaël Badin
Update: power_types being a set, allenap advised using power_types.discard. -- https://code.launchpad.net/~rvb/maas/pserv-power-fix/+merge/106609 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/pserv-power-fix into lp:maas.

[Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad

2012-05-21 Thread James Tunnicliffe
James Tunnicliffe has proposed merging lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1002257 in Launchpad itself: postponed done upcomingwork

Re: [Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad

2012-05-21 Thread Curtis Hovey
Review: Approve code Thank you. I think 'pu' is ambiguous. Is there a better name like 'package_upload' that will make this code easier to read? -- https://code.launchpad.net/~stevenk/launchpad/populate-spph-pu/+merge/105601 Your team Launchpad code reviewers is subscribed to branch

[Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad

2012-05-21 Thread Curtis Hovey
The proposal to merge lp:~stevenk/launchpad/populate-spph-pu into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~stevenk/launchpad/populate-spph-pu/+merge/105601 --

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread Colin Watson
Colin Watson has proposed merging lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #1001517 in Launchpad itself: generate-extra-overrides can leave stale files behind in config.germinateroot

[Launchpad-reviewers] [Merge] lp:~allenap/maas/database-run into lp:maas

2012-05-21 Thread Gavin Panella
Gavin Panella has proposed merging lp:~allenap/maas/database-run into lp:maas with lp:~allenap/maas/convert-enums-redux as a prerequisite. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see:

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/database-run into lp:maas

2012-05-21 Thread Gavin Panella
ClusterFixture now comes from postgresfixture. -- https://code.launchpad.net/~allenap/maas/database-run/+merge/104759 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/database-run into lp:maas. ___

Re: [Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad

2012-05-21 Thread Benji York
Review: Approve code This branch looks good. I have a couple of small page template suggestions: The divs that is used to tal:define the classname string will show up in the generated output, which won't kill kittens or anything, but it can be avoided by using a tal-namespaced element instead,

Re: [Launchpad-reviewers] [Merge] lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad

2012-05-21 Thread j.c.sackett
The changes look good, but this branch is missing tests. lib/lp/app/browsers/tests/test_formatters.py was not updated. The renaming of public_private_css = global_css should have broken the existing tests. I expect to see tests for the new rules. I am surprised that the change of

[Launchpad-reviewers] [Merge] lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad

2012-05-21 Thread j.c.sackett
The proposal to merge lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad has been updated. Commit Message changed to: Updates the banners to use progressive enhancmenet, so they are still shown without JS. For more details, see:

Re: [Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad

2012-05-21 Thread Benji York
Review: Approve code This branch looks good. Here's one tiny thing you might want to tweak: The original code included superfluous calls to float. Since multiplication and division have the same priority in order of operation and there is a float (100.0) as the first operand, we don't need the

[Launchpad-reviewers] [Merge] lp:~rvb/maas/celery-tests into lp:maas

2012-05-21 Thread Raphaël Badin
Raphaël Badin has proposed merging lp:~rvb/maas/celery-tests into lp:maas. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~rvb/maas/celery-tests/+merge/106639 This branch adds a Celery fixture to be able to test celery tasks

[Launchpad-reviewers] [Merge] lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad

2012-05-21 Thread Curtis Hovey
The proposal to merge lp:~jcsackett/launchpad/progressively-enhanced-banners into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~jcsackett/launchpad/progressively-enhanced-banners/+merge/106487 --

[Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad

2012-05-21 Thread Benji York
The proposal to merge lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~dooferlad/launchpad/upcomingwork_show_incomplete_bp/+merge/106438 --

[Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad

2012-05-21 Thread Benji York
The proposal to merge lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~dooferlad/launchpad/postponed-is-done/+merge/106611 --

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Gavin Panella
Thanks for doing this. Not a full review (yet), but a note about the changes in test_js.py: ... This is pretty hard to follow, and it indents into dizzying heights. I've moved get_failed_tests_message to a new module, maastesting.yui3, and decomposed it into smaller, easier to understand

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Gavin Panella
This foot-shooter in src/maastesting/utils.py is a bit cryptic: 1167+def preexec_fn(): 1168+# Revert Python's handling of SIGPIPE. See 1169+# http://bugs.python.org/issue1652 for more info. 1170+signal.signal(signal.SIGPIPE, signal.SIG_DFL) Can you explain its

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Gavin Panella
More loose notes: YUIUnitBase could do with some documentation. What is the role and purpose of cloning? Good point, added. -- https://code.launchpad.net/~allenap/maas/try-out-saucelabs/+merge/106217 Your team Launchpad code reviewers is requested to review the proposed merge of

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Gavin Panella
More notes follow: YUIUnitTestsRemote could also do with some documentation. For example, what happens when I run tests without a connection to the SauceLabs site? Erm, it'll break. It's not designed to run without a connection to SauceLabs, and doesn't allow you to even try it. I'll see

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Gavin Panella
In src/maastesting/tests/test_httpd.py: 736 +def test_use(self): 737 +filename = setup.py 738 +self.assertThat(filename, FileExists()) 739 +with HTTPServerFixture() as httpd: 740 +url = urljoin(httpd.url, filename) 741 +

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread Benji York
Review: Approve code Looks good. The with/pass in test_generate_extra_overrides.py (line 877 of the diff) confused me for a second. I wonder if an old-fashioned open().close() wouldn't be easier to understand at first blush. Or maybe even a touch helper function. --

Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread Colin Watson
Maybe I'm weird, but I prefer the with/pass pattern myself; however, a helper function makes sense, and perhaps can be moved somewhere more common in future. Done. -- https://code.launchpad.net/~cjwatson/launchpad/germinate-stale-files/+merge/106626 Your team Launchpad code reviewers is

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread Benji York
The proposal to merge lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~cjwatson/launchpad/germinate-stale-files/+merge/106626 --

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread Colin Watson
The proposal to merge lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad has been updated. Commit Message changed to: Remove stale files from config.germinateroot. For more details, see: https://code.launchpad.net/~cjwatson/launchpad/germinate-stale-files/+merge/106626 --

[Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad

2012-05-21 Thread noreply
The proposal to merge lp:~dooferlad/launchpad/postponed-is-done into lp:launchpad has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~dooferlad/launchpad/postponed-is-done/+merge/106611 --

[Launchpad-reviewers] [Merge] lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad

2012-05-21 Thread noreply
The proposal to merge lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~dooferlad/launchpad/upcomingwork_show_incomplete_bp/+merge/106438 --

[Launchpad-reviewers] [Merge] lp:~benji/launchpad/bug-998040 into lp:launchpad

2012-05-21 Thread Benji York
Benji York has proposed merging lp:~benji/launchpad/bug-998040 into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~benji/launchpad/bug-998040/+merge/106697 Bug 998040 describes an issue that has been fixed in

Re: [Launchpad-reviewers] [Merge] lp:~benji/launchpad/bug-998040 into lp:launchpad

2012-05-21 Thread Gary Poster
Review: Approve -- https://code.launchpad.net/~benji/launchpad/bug-998040/+merge/106697 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to :

[Launchpad-reviewers] [Merge] lp:~benji/launchpad/bug-998040 into lp:launchpad

2012-05-21 Thread Gary Poster
The proposal to merge lp:~benji/launchpad/bug-998040 into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~benji/launchpad/bug-998040/+merge/106697 -- https://code.launchpad.net/~benji/launchpad/bug-998040/+merge/106697 Your

[Launchpad-reviewers] [Merge] lp:~wallyworld/launchpad/distro-page-ajax-835020 into lp:launchpad

2012-05-21 Thread Curtis Hovey
The proposal to merge lp:~wallyworld/launchpad/distro-page-ajax-835020 into lp:launchpad has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~wallyworld/launchpad/distro-page-ajax-835020/+merge/106321 --

[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad

2012-05-21 Thread noreply
The proposal to merge lp:~cjwatson/launchpad/germinate-stale-files into lp:launchpad has been updated. Status: Approved = Merged For more details, see: https://code.launchpad.net/~cjwatson/launchpad/germinate-stale-files/+merge/106626 --

[Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/builder-description-mandatory into lp:launchpad

2012-05-21 Thread Steve Kowalik
Steve Kowalik has proposed merging lp:~stevenk/launchpad/builder-description-mandatory into lp:launchpad. Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: Bug #984342 in Launchpad itself: description field for builder is optional but mandatory for DB

Re: [Launchpad-reviewers] [Merge] lp:~allenap/maas/try-out-saucelabs into lp:maas

2012-05-21 Thread Julian Edwards
On Monday 21 May 2012 16:27:19 you wrote: By the way, according to Clarkson this should get you shot: 345 === added file 'src/maastesting/httpd.py' 346 --- src/maastesting/httpd.py1970-01-01 00:00:00 + 347 +++ src/maastesting/httpd.py2012-05-17 16:13:19

Re: [Launchpad-reviewers] [Merge] lp:~stevenk/launchpad/builder-description-mandatory into lp:launchpad

2012-05-21 Thread William Grant
Review: Disapprove code As discussed on IRC, the column should probably be dropped instead. -- https://code.launchpad.net/~stevenk/launchpad/builder-description-mandatory/+merge/106729 Your team Launchpad code reviewers is subscribed to branch lp:launchpad.