Brock Pytlik wrote:
> https://cr.opensolaris.org/action/browse/pkg/bpytlik/7139873-v1
bootenv.py:
- I think we need some brief documentation explaining what the three
image variables are, and how they interact. Getting it right is
critical, and having to figure out what needs to happen by reading the
code is difficult and error-prone at best.
- line 75: space before equals. I'd also call this img_set_cb() or
something that indicates that it's a callback function.
- line 148: I know that the one place that uses this mechanism does it
correctly, but in the general case, is there a real requirement for
img_setter to be non-None? Why not just call it only if it's non-None?
- line 152: would it be correct and not confusing to have current_img
simply return self.clone_img when that was not None, and self.orig_img
otherwise?
- line 224: do we need this assert on img_setter here, too (even if we
keep the other)?
- line 431: if this is the case, then why bother passing img as a
parameter?
- line 432: use "is"
image.py:
- line 259: space after colon
- line 261: probably just clone()
imageplan.py:
- line 123: space after colon
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
