[merge] Registry

[Martin Pool]

On 12/10/2006, at 23:41 , Aaron Bentley wrote:

> Martin Pool wrote:
>
>> +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.
>
> Can we also get hardcore about apostrophes?  There's a certain core  
> dev
> who often omits them, and I always find that contractions which are
> missing their apostrophes are quite jarring.

Let's get hardcode about good style for documentation/comments in  
general.

> "dont" and "cant" are
> acceptable in symbol names, but I don't think they belong in  
> doctexts or
> comments.  (Though "cant" is a real word that means something else.)

Actually I find them a little distasteful in identifiers, and would  
either remove the negative (thing.can_eat_peanuts), or expand it  
(CannotDance).  The second is a bit harder to justify though.

> Getting more serious about grammar will make it a bit harder to merge
> patches.  We might want to consider making integrators take
> responsibility for tweaking grammar, because I don't want this to  
> become
> an extra hoop for non-native English speakers to jump through.

Well, one thing we can do is to give review comments but say "but  
I'll fix all of these when I merge them" -- this can apply to either  
code or doc changes.  Then other people know what was done and the  
original author gets the feedback, but we don't need to block.

If I'm reviewing English style or spelling I do try to give an  
alternative text.  Sometimes this just shows that I misunderstood the  
original and it can be more clear.

>>> +    This is used by Registry to make plain objects function  
>>> similarly
>>> +    to lazily imported objects.
>
> I'd be inclined to use hyphens for compound words like
> "lazily-imported", which makes the binding crystal-clear.

The other day I saw an ad for a dress which said it was "ideal for  
lunch with the girls or after five drinks."

>>> +class _LazyObjectGetter(_ObjectGetter):
>>> +    """Keep a record of a future object, and on request load it."""
>
> Should be "and, on request, load it."

I don't think it's incorrect as originally written, though yours  
might be clearer.  Sometimes I try to say "may be clearer as".

More generally imperatives like this are appropriate for functions/ 
methods, but less so for classes.  So it might be better as "Keeps a  
record".

>>> +    def get_obj(self):
>>> +        """Get the referenced object.
>>> +
>>> +        If not imported yet, this will import the requested module.
>
> This has the object of the sentence hidden at the end of it.  I'd
> prefer: "This will import the requested module, if not yet imported"

+1
>>> +        :param help: Help text for this entry. This may be a  
>>> string or
>>> +                a callable. If it is a callable, it should take two
>>> +                parameters, this registry and the key that the  
>>> help was
>>> +                registered under.
>
> This should have a colon rather than a comma after 'parameters',
> because the list 'this registry and the key...' describes  
> 'parameters'.

+1
>>> +        :param info: More information for this entry.  
>>> Registry.get_info()
>>> +                can be used to get this information. It is meant  
>>> as an
>>> +                opaque storage location.
>>> +        :param override_existing: If True, replace the existing  
>>> object
>
> Should this be "If True, replace any existing..." ?
>
> Also, "is meant as" might be better as "should be treated as".
>
>>> +    def register_lazy(self, key, module_name, member_name,
>>> +                      help=None, info=None,
>>> +                      override_existing=False):
>>> +        """Register a new object to be loaded on request.
>>> +
>>> +        :param module_name: The python path to the module. Such  
>>> as 'os.path'.
>>> +        :param member_name: The member of the module to return,  
>>> if empty or
>>> +                None get() will return the module itself.
>
> This is a comma splice.  A semicolon, if not a period, should be here,
> because there are two thoughts.  I'd write:
>
> :param member_name: The member of the module to return.  If empty or
>         None, get() will return the module itself.

Right.  It's not so much about where there are commas as untangling  
the conditionals.


-- 
Martin