John Arbash Meinel
john at arbash-meinel.com
Fri Oct 13 01:30:07 BST 2006
-----BEGIN PGP SIGNED MESSAGE-----
Martin Pool wrote:
> On 27 Sep 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
>> Robert Collins wrote:
>>> On Thu, 2006-09-21 at 16:28 -0500, John Arbash Meinel wrote:
>>> John and I talked on IRC and we are mostly in agreement now... sometime
>>> soon should see a new patch :)
>> Attached is an updated Registry, which defers to helper classes for
>> handling whether to lazy import or not.
>> And it lessens how much Registry tries to look like a dictionary.
> +1, except for a couple of grammatical fixes. You have a habit of
> separating your clauses into sentences, as in this quoted section. I
> don't mind how you write your email, but we should try to stick to
> standard English in documentation, and I'm pretty sure this isn't
> (It's only because the abstraction is so elegant already that I mention
> this. Perhaps it sounds trivial but we might as well get it right)
I guess I haven't really thought about the English side of things. But I
admit that I tend to communicate in clauses. I have no problem with
corrections. (Now that you have made me self-conscious of it, I'm not
sure how to structure sentences anymore :)
>> +class _ObjectGetter(object):
>> + """Hang onto an object, and return it on request.
> 'on to'
"Maintain a reference to an object, and return the object on request"
>> + This is designed such that you can register objects in a lazy fashion,
>> + so that they can be imported later. While still having the help text
>> + available right away.
>> + """
> 'be imported later, while still having'
> I would like for something to be said about the default_key in the class
>> + def get(self, key=None):
>> + """Return the object register()'ed to the given key.
>> + May raise ImportError if the object was registered lazily and
>> + there are any problems, or AttributeError if the module does not
>> + have the supplied member.
> May be better as
> :raises: ImportError if the object was registered lazily and
> there are any problems
> :raises: AttributeError if the module does not
> have the supplied member.
^- Thanks, I only learned about :raises: recently, and it certainly is
the correct item here. Though I thought the syntax was:
>> + default_key = property(_get_default_key, _set_default_key,
>> + doc="Current value of the default key."
>> + "Can be set to any existing key.")
> OK but why?
^- Why can it be set to any existing key, or what does it actually do?
> Can we also add something to HACKING on the lines of
> Several places in Bazaar use (or will use) a registry, which is a
> mapping from names to objects or classes. The registry allows for
> loading in registered code only when it's needed, and keeping
> associated information such as a help string or description.
> just to point folks in the right direction.
I'll go over the other review comments (I'm not sure why I got Martin's
response before I got Aaron's).
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar