Shawn Walker wrote:
> http://cr.opensolaris.org/~swalker/pkg-11522-2-full/
Looks pretty good; these are mostly nits.
client.py:
- line 163: join with line 164?
- Did we really agree on requiring --all instead of just defaulting to
that if we don't get a publisher prefix?
- line 2411: is the check for repo_uri necessary? Given the previous if
clause, doesn't repo_all imply repo_uri?
- line 2464: since this else clause exits the function regardless, why
not make it the if clause (if not repo_uri:) and pull the other clause
back into the main flow of the function?
- line 2482: Won't this happen on many older depots? Or do you get back
something sane from the API when you connect to one?
- line 2523: can this be pulled out and shared between the two clauses?
- line 2615: "taht" -> "that"
api.py:
- line 2655: why would a stat return EROFS?
api_errors.py:
- line 1517: "encounetered" -> "encountered"
- line 1518: "message" -> "exception"?
image.py:
- line 618: I'd make is_zone False by default.
- line 669: "nimimise" -> "minimize"
- line 882: Maybe I'm paranoid, but isn't this test backwards? That is,
it returns if the imagedir is not /, but we want it to return when it
is, right?
t_pkg_depotd.py:
- line 576: I thought generally we wanted to have depot URL roots have a
trailing slash.
pkg5unittest.py:
- line 1482: won't this die after you just rmtree()d dest?
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
