Jack,
Thanks for the review, comments below :
On 12/15/10 07:43 PM, Jack Schwartz wrote:
Hi Matt.
Thanks for doing this work. Here are my comments:
parser.py:
106-107: I would remove comment about DerivedManifest using this. IMHO
users of a method shouldn't be listed in the method's comments, as the
comment can sneakily go out of date.
Will remove any reference to DerivedManifest.
Consider putting 393-398 under the if of 390. No need to continuously
check that the manifest exists and is a file after doing it once.
Actually it's needed where it is. These checks used to be performed in
__init__() when the manifest argument was required.
However as they are not being done in the __init__() anymore they are
required here, to check for __init__() argument as well as for doc
retrieved manifest.
The check for _manifest is None should always be done, however the check
for os.path.isfile(), if moved under the if as you recommend would have
to be added back to the __init__() if the manifest argument was not None.
As it is having it in one place to be makes more sense, and it's very
little overhead.
428: I think can_handle() has to return False instead of None here.
While None may work, I see the DOC calls it and expects a boolean.
(lib/install_doc/data_object/cache.py line 349). That same file also
implements a can_handle() method which does the same as yours, and
returns False for the same reason
that ManifestParseData's returns None:
def can_handle(cls, xml_node):
'''The DataObjectCache class doesn't import any XML itself.'''
return False
Will change to return False.
The other files look fine.
thanks
Matt
Thanks,
Jack
On 12/15/10 09:32 AM, Matt Keenan wrote:
Hi,
I'd like a couple of pair's of eyes to scan over a fix for bug :
7004864 Read profile to be parsed from DOC if not specified via arguments
http://monaco.sfbay.sun.com/detail.jsf?cr=7004864
Webrev ;
http://cr.opensolaris.org/~mattman/bug-7004864/
This request modifies Manifest Parser so that a manifest being passed
at initialization is not a hard requirement. If not passed it will be
read from a location in the Data object Cache. This functionality is
being added to support Derived Manifests.
cheers
Matt
_______________________________________________
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
