[merge] Registry

John Arbash Meinel john at arbash-meinel.com
Fri Oct 13 01:30:07 BST 2006


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

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 :)
>>>
>>> -Rob
>> 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
> standard.
> 
> (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'

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

Updated.

...

>> +    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:
:raises ImportError:
not
:raises: ImportError


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

Certainly.

I'll go over the other review comments (I'm not sure why I got Martin's
 response before I got Aaron's).

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFLt4OJdeBCYSNAAMRAoHHAKDQ96BTVCOm4Bom0BW4JQGysfvF6QCgiDOG
R0BnmDHN8BISzqNHSCfDD9Y=
=X+Zs
-----END PGP SIGNATURE-----




More information about the bazaar mailing list