On 01/25/11 03:54 PM, Jack Schwartz wrote:
HI Jan.

On 01/25/11 04:37 AM, Jan Damborsky wrote:
Hi Jack,

thank you very much for review.
Please see my response in line.

Jan


On 01/24/11 10:46 PM, Jack Schwartz wrote:
Hi Jan and Keith.

I've re-examined just the parts I commented on earlier. Most looks good. Just a few comments:

profile/__init__.py:

117: I didn't think setters were supposed to return anything. Should it raise an exception to denote an
error condition?

From what I have just learnt about Python 'setter', its return value seems to be discarded so unless Keith objects, I will simplify code on lines 116-119
in following way:

if self.system and user_infos:
  self.system.users = user_infos
My only concern here is a silent failure where nothing is set and no error is generated. Will this be a problem?

    Thanks,
    Jack

Actually, the original code was probably correct, now that I think of it. It would error in that case...

- Keith




74: In a previous response, Keith suggested that "there was a lack of docstrings in the ConfigProfile object. There are no newer comments in that object, but I'm not sure that any are needed there at this point... What do you think?

I will let Keith comment on that aspect.



_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to