On 09/15/11 08:12 AM, Darren Kenny wrote:
Hi Jack,
Generally good :)
Some minor comments:
- mim.py:
- lines 423 and 582, this could probably be written as:
if not path or not value:
Yes, thanks. Fixed.
- line 468 - why did you change this, the logic should be the same.
A string will fail 'true-ness' if it's None or it's an empty string.
Fixed to if not path.
- line 676-677:
Is it possible that there could be zero values in right_set?
No, since the final branch may not have a value, and we don't accept an
empty path string. It was verifying this that led me to test with an
empty string, which in turn led to changing the "if not path" lines
commented on above.
I have augmented the comment around 619 to explain this.
- process_dtd.py
- line 787:
Is it true that the parent path is used only once in the tree?
The comment at ~767 in the code was not clear. I have changed it to:
"Assumes that each element tag is referenced only once in the DTD."
If you have better wording please chime in...
Certainly in the AI manifest it's possible to have some repeated tags,
but maybe this refers only to the DTD?
Hopefully above clarifies.
Thanks,
Jack
Thanks,
Darren.
On 15/09/2011 03:59, Jack Schwartz wrote:
Hi everyone.
Seeking two reviews by Thursday COB and Dave's approval please. I'd like to
request Ethan be one of the reviewers.
Here is a code review that fixes a hole in aimanifest path processing. Without
this bugfix,
aimanifest add diskname@name c0t0d0s0
or other references to non-absolute paths which have no values in them fails.
webrev:
https://cr.opensolaris.org/action/browse/caiman/schwartz/7090604_1
bug report:
http://monaco.us.oracle.com/detail.jsf?cr=7090604
While it introduces a new method to find the path leading to the referenced
element, all code introduced by this fix affects only the failing cases;
codepaths of working cases are not affected.*
* While in there, I added some additional args checking for set(), get() and
add(), which I have tested but can remove if it makes the difference between
being allowed in or not.
Thanks for your time,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
