Hi Keith.
Wrapping up loose ends:
On 04/11/11 09:09 AM, Keith Mitchell wrote:
280-281: I don't see a strict need to check this here. There are any
number of invalid parameters that could be passed in (True, False,
float, int...); rather than selectively catching just None, it would
be more "pythonic" to just let parse_xml_file - or rather,
etree.parse - choke on them. Python's about letting the caller
figure out if what it passes in is valid or not!
My library API enforces a more strict specification than what the
lower-level libraries allow. The lower-level libraries allow a None
value where I don't. That is why I test for it in some places. In
other places I test to be consistent with the other places where it
is needed.
Ok. Would it be worth adding a comment explaining that, for those
cases? I wouldn't want someone (say, me) to come along in a year, fix
a bug, and "clean up" these checks, if they're required. e.g.:
# parse_xml_file() accepts "None" as a filename, but that is invalid
here (because XYZ)
(Or am I too paranoid?)
Added comments to set() and add() for this.
[...]
626: Nit: "if insert_before is None"
This code has to be this way. It needs to differentiate an empty
list from no list at all.
Fair enough!
... and I added a comment about this in the code.
Thanks again,
Jack
Thanks again for your review.
Webrev respun:
delta:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.cli.3.4.diff
vs slim_source:
http://cr.opensolaris.org/~schwartz/110329.1/webrev.cli.4
Jack
Don't see anything else. Thanks for making the changes.
- Keith
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
