Code review comment for lp:~mwhudson/launchpad/bzr-2.1c1-update

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Hi there,
>
> This branch updates Launchpad to bzr 2.1rc1. It's mostly very boring fallout from bzrlib's Server class setUp -> start_server renaming although there a few test infrastructure and logging related changes too.
>

Yes, this was actually kind of boring but I managed to stay awake ... ;-)

As a consequence, I only have one marginal suggestion. But if you don't
feel like doing something about it, feel free to land this branch ... ;)
Thank you for doing this boring work!

 review approve code

Cheers,
Henning

> === modified file 'lib/lp/codehosting/scanner/branch_scanner.py'
> --- lib/lp/codehosting/scanner/branch_scanner.py 2009-12-17 02:00:16 +0000
> +++ lib/lp/codehosting/scanner/branch_scanner.py 2010-02-01 05:13:16 +0000
> @@ -23,7 +23,7 @@
> from lp.codehosting.scanner.bzrsync import (
> BzrSync, schedule_diff_updates, schedule_translation_upload)
> from lp.codehosting.scanner.fixture import (
> - Fixtures, make_zope_event_fixture, run_with_fixture)
> + Fixtures, make_zope_event_fixture, run_with_fixture, ServerFixture)
> from canonical.launchpad.webapp import canonical_url, errorlog
>
>
> @@ -100,7 +100,8 @@
> schedule_translation_upload,
> ]
> server = get_scanner_server()
> - fixture = Fixtures([server, make_zope_event_fixture(*event_handlers)])
> + fixture = Fixtures(
> + [ServerFixture(server), make_zope_event_fixture(*event_handlers)])

Wouldn't it be cool to be able to just do "IFixture(server)" here? See
below.

> self.log.info('Starting branch scanning')
> branches = getUtility(IBranchScanner).getBranchesToScan()
> run_with_fixture(fixture, self.scanBranches, branches)
>
> === modified file 'lib/lp/codehosting/scanner/fixture.py'
> --- lib/lp/codehosting/scanner/fixture.py 2009-06-30 16:56:07 +0000
> +++ lib/lp/codehosting/scanner/fixture.py 2010-02-01 05:13:16 +0000
> @@ -1,4 +1,4 @@
> -# Copyright 2009 Canonical Ltd. This software is licensed under the
> +# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> # pylint: disable-msg=E0211
> @@ -16,6 +16,7 @@
> 'IFixture',
> 'make_zope_event_fixture',
> 'run_with_fixture',
> + 'ServerFixture',
> 'with_fixture',
> ]
>
> @@ -121,3 +122,18 @@
>
> def make_zope_event_fixture(*handlers):
> return Fixtures(map(ZopeEventHandlerFixture, handlers))
> +
> +
> +class ServerFixture:
> + """Adapt a bzrlib `Server` into an `IFixture`."""
> +
> + implements(IFixture)

adapts(...)

Maybe this could be registered as a real adapter? But I am not sure if
there is an interface available that could be adapted.

> +
> + def __init__(self, server):
> + self.server = server
> +
> + def setUp(self):
> + self.server.start_server()
> +
> + def tearDown(self):
> + self.server.stop_server()
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAktmxtgACgkQBT3oW1L17iiaIACgv/EpHTRVrmySnKnWMpawunBD
IykAoMkEW1ptPuRpzcPr+1ETxMfNHBcM
=aQMJ
-----END PGP SIGNATURE-----