[MERGE] Refactor cmd_serve to a be easier to extend.

Jonathan Lange jml at mumak.net
Fri Jan 23 22:49:45 GMT 2009


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

On Fri, Jan 23, 2009 at 8:27 PM, John Arbash Meinel  wrote:
>> This patch has no behavioural changes, it just explodes
>> cmd_serve.run() into a few bits so that it's easier for other plugin
>> authors to extend and/or reuse.
>>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: http://getfiregpg.org

iEYEARECAAYFAkl6SYkACgkQX+0xPtMTonwntQCfcV5GjyHR0jQDitvJPgIDxaN5
/QsAn3ycFj2vKWfu64N77XeFAQY0yL+j
=eptI
-----END PGP SIGNATURE-----

...
> +            smart_server = medium.SmartServerPipeStreamMedium(
> +                sys.stdin, sys.stdout, t)
>
> ^- I don't see a variable "t" in the function here. I think you want
> "transport" at this point.
>

Fixed.

> v- and here
> +        else:
> +            host, port = self.get_host_and_port(port)
> +            smart_server = server.SmartTCPServer(t, host=host, port=port)
> +            note('listening on port: %s' % smart_server.port)
>

And here too.

>
> Do tests actually pass after your refactoring? Because that certainly
> seems like a gap in coverage if we never test that "run_bzr('serve')"
> and "run_bzr('serve --inet')" actually work. (They may need to be
> run_bzr_subprocess, and directly specify the URL to bind to, etc.
>

They didn't. I forgot to run them before submitting.

Here's the updated patch that fixes all the tests.

Thanks for the quick review,
jml
-------------- next part --------------
A non-text attachment was scrubbed...
Name: refactor-serve-2.patch
Type: text/x-diff
Size: 13150 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090123/031f5561/attachment-0001.bin 


More information about the bazaar mailing list