Hello Alec,

On Oct 30, 5:45 pm, "Alec Thomas" <[EMAIL PROTECTED]> wrote:
> Hey Christian,
>
> Fantastic work cleaning this stuff up, looks like it was a lot of work.
>
> I've only looked at the Resource class itself, and have some comments:
>
>   - In general I think having four different ways of creating resources
>     is both redundant and confusing:
>
>       - Resource(realm, id, version)
>       - Resource.from_spec(realm, id, version)
>       - Resource.from_spec(resource)
>       - Resource.from_spec((realm, id, version)).
>
>     This is extreme overkill IMO.

extreme overkill is perhaps itself an overstatement ;-)

>
>     The tuple form in particular can be replaced with argument expansion
>     (*mytuple).

We don't actually need the tuple form, that was just a temporary need.
I'll clean that up.


>     From looking at the usage of from_spec() it seems that the sole
>     reason for its existence is as a convenience so you can pass in a
>     resource *or* a realm and have a Resource object returned.

Yeah, it's nothing more than that, used in both the Context and the
PermissionCache.


>     Is it a big deal if we put this logic into the constructor? The only
>     difference will be that it will always return a new object rather
>     than sometimes returning the resource object passed in.
>
>     This would leave us with:
>
>       - Resource(realm, id, version)
>       - Resource(resource)

Well, the only purpose of Resource.from_spec was to avoid the
Resource(resource) case and the creation of an unneeded object.
But there's a simple trick here to return resource itself in the
second case, so it might be worth doing it.


>     Which I think is a lot cleaner and simpler.
>
>   - I'd prefer it if Resource.__call__() was renamed to clone, duplicate or
>     copy or something.

Any would be fine for me, like copy(). OK, then we need to do the same
on Context. I didn't start in that direction because I wanted to keep
some degree of backward compatibility with 0.11dev, but that
compatibility mode is now gone anyway, so...

-- Christian


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac 
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to