Code review comment for lp:~vila/bzr/move-test-servers

On 11 February 2010 14:06, Robert Collins <email address hidden> wrote:
> On Thu, 2010-02-11 at 02:51 +0000, Martin Pool wrote:
>>
>> > Perhaps bzrlib.transport.servers.* ?
>>
>> I'd slightly prefer .servers:
>>
>> 1- submodules seem to occasionally cause more annoying python import
>> behaviour (circularity etc) than sibling modules
>
> Could you enlarge on this? I haven't seen that, unless you mean the
> behaviour of 'import foo' in a submodule trying for
> 'parent.thispackage.foo' first, rather than 'foo.' (the top level foo).
> This should be pretty fast after it is tried once, because after that
> the dentry cache is hot.

You can't do 'import bzrlib.ui.text' in top-level code in 'bzrlib.ui'.

>> 2- bzrlib.transport.servers seems a bit longwinded
>
> I proposed transport.servers because:
>  - They are specifically transport servers, not smart server servers, or
> git servers, or CVS servers; and I wouldn't really want to see a git
> server mixed in. I guess we could do servers.transports.*, but thats
> equally long winded, and IMO less clear.

Are they really? They are things that can be started, stopped, and
that have a URL. Their essentially connection to transports (as
opposed to the fact that they are only used for testing transports)
seems quite weak.

I would be ok with having bzrlib.server.ftp alongside
bzrlib.server.git for example.

>  - I see the transport servers as being generally coupled to transports,
> not coupled to bzrlib as a whole.
>
> That said, its up to whomever proposes the patch :)

--
Martin <http://launchpad.net/~mbp/>