Hi Darren,
I just looked at the new path related functions.
Couple minor notes:
install_common/__init__.py: Unless there's a valid reason for importing
every module under solaris_install in one go, there's no need to define
__all__ here.
data_object/__init__.py:
589: "if" block shouldn't be necessary, if the "for" block is re-worked
slightly:
parent = self
while parent is not None:
my_path.insert(1, quote(parent._name, ""))
parent = parent._parent
681: use "if 'attribute' in kwargs" rather than a try/except block here
696/712: Unnecessary, if the exception will be merely reraised, don't
bother catching it in the first place.
745-6: Nit: Might be slightly more pythonic if you used
"class_name.split('.')" and then parsed based on the length of the
result (>1 = fully qualified).
755-759: Would it be worth it to attempt to import the module, or no?
762: Use hasattr rather than getattr and a try/except block here.
The function doesn't appear to descend further (i.e., referencing
"a.b.c" would seem to break it).
776: nit: If there's space on the line to do away with the "match"
variable (and just use "if
DataObjectBase.__NAME_RE.match(value_string)), that'd be preferred
(perhaps changing the variable from "value_string" to something slightly
shorter).
Thanks,
Keith
On 09/ 7/10 07:05 AM, Darren Kenny wrote:
Anyone able to provide some review of this code?
Thanks,
Darren.
On 08/30/10 05:23 PM, Darren Kenny wrote:
Hi,
After the last code review, several things changed, which were:
- Relocation of package from osol_install.data_object_cache to
solaris_install.data_object
- Renaming of python files as:
data_object_cache/data_object.py
-> data_object/__init__.py
data_object_cache/data_object_cache.py
-> data_object/cache.py
data_object_cache.data_object_dict.py
-> data_object/data_dict.py
- Restructuring of objects to correct for better OOD to have the
DataObjectBase class without deletion and insertion methods, which
is a base class for DataObject, DataObjectCache and DataObjectDict.
DataObject adds these methods, and is still the preferred object for
people to sub-class.
(See attached image for UML diagram - also in design document).
- Addition of the 'paths' feature, which added:
- object_path - read-only property
- find_path(path) - DataObjectBase method.
- Addition of pydoc comments to all tests.
I've updated the latest webrev at:
http://cr.opensolaris.org/~dkenny/install_doc/
Unfortunately producing a diff between webrevs isn't all that useful due to the
above changes.
I would appreciate if any feedback could be provided by the end of the week.
Many Thanks,
Darren.
_______________________________________________
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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
