Copying to caiman-discuss. Jean
-------- Original Message -------- Subject: Re: DDU code review? Date: Fri, 19 Feb 2010 11:18:44 -0700 From: jeanm <[email protected]> To: Jack Schwartz <Jack.A.Schwartz at Sun.COM> On 02/18/10 12:10 PM, Jack Schwartz wrote: > Thanks, Jean. > > See below. > > jeanm wrote: >> On 02/18/10 11:46 AM, Jack Schwartz wrote: >>> Hi Jean. >>> >>> Are you available to help with this? Please let me know either way. >> >> Sure. What part? > AI library: > http://cr.opensolaris.org/~billyan/DDU_1.3.1/webrev/ddu_1.3.1_AI_webrev/ Here's my comments for the AI library. I'll get to the UI stuff next. General: Make sure you run these files through pylint src/AI/ddu_devdata.py: You have status, output =<whatever> in several places but status isn't use. Doesn't pylint complain about this. src/AI/ddu_function.py: Can you please indent lines 61 and 62? line 62: remove the double repo_obj_list When you're scanning command output and splitting on it etc, it might be nice to have a comment showing what the output looks like in order to make the code more understandable. This applies to ddu_devdata.py too. lines 134& 135: please indent for readability. line 134: spelling on controller needs fixing. lines 138-284: Would it be more readable to have the if/else clauses just set the appropriate fields and then have 1 call to drv_list.append? line 291-391: It would be nice to have some comments in this code to explain what you're doing. src/data/driver.db: Please update the copyright and remove the ident string. src/data//driver_a.db: Please update the copyright. src/data/hd.xml: Please update the copyright and remove the ident string. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20100219/42c25150/attachment.html>
