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

On Thu, 2010-02-11 at 03:45 +0000, Martin Pool wrote:
> 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'.

Thats true. That doesn't seem to apply here though, does it? I don't
think we should shun all subpackages and submodules because that can
happen. In fact, its the case in all imports that you can't import from
within the currently importing package at the top level - this is why
circular imports are a problem. bzrlib.servers seems no more or less
inclined to suffer this than bzrlib.transport.servers, as in neither
case do we seem to have an a-priori need to expose things from within
the namespace.

> >> 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.

They are all servers for VFS's, rather than servers for non-FS-like file
systems.

-Rob