Brock Pytlik wrote:

> Danek Duvall wrote:
> >Brock Pytlik wrote:
> >
> >>http://cr.opensolaris.org/~bpytlik/ips-11805-v1/
> >
> >__init__.py:
> >[snip]
> >
> >  - I don't understand why you haven't simply enhanced fromstr() and
> >    fromlist() to take the optional basedirs and load_data arguments.
> I think because I view these as fundamentally different operations.

Huh.  I really don't.  They're both constructing an action from a string or
a list of strings, but one is abstract, disconnected from any payload,
while the other isn't.  They seem quite close to me.

> I think that's something we want to separate as there's potential for a
> fair amount of confusion. I, at least, certainly took a long time to get
> the details straight in my head, and I think conflating the
> representations within fromstr would add to, not remove, that confusion.

Can you describe the confusion?  It seems pretty straightforward to me, but
not having had to dive into this to fix the bug, I haven't gotten up close
and personal with the issue.

> Also, it greatly reduced the potential impact, especially the performance
> impact, of my change since many, many places in the code call fromstr
> while only pkgsend and pkgdep use the functionality of internalizestr.

I don't see how the performance impact would be all that great.  You
already have a boolean argument to decide whether or not to do any payload
management, so it amounts to a single extra test.

Still, if you're set on separate functions, could you at least explain what
"internalize" means?  Why not fromstr_payload() or something?

> >[snip]
> >
> >manifest.py:
> >
> >  - this functionality is really useful, but I'd like to see it implemented
> >    differently -- drop actions_set_content(), but allow the "content"
> >    parameter to set_content() to have multiple types.
> I think we've had this conversation before, but I think having functions
> which accept types with materially different interfaces (ie, the code
> basically has to do isinstance/issubclass), leads to code which is harder
> to maintain and use. (Maybe I've simply missed the right way to handle
> this, but I'm pretty sure such a check will need to be made.) That said,
> I guess this is the "pythonic" way of doing things, so I'll switch to it.

Thanks.  I don't think the maintenance burden is high, and I really hate
having to pretend we're C and have a mass of slightly differently named
functions based on what kind of parameters we're passing in.  If we had
language built-in support for function polymorphism based on argument
types, we'd just use that, but that's not possible to do in Python.

> (If the objection is to having the initialization duplicated, I'm happy
> to break that out into a common function as well, I just hadn't thought
> of it till now.)

That's part of it, but only a small part.

Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to