On Thu, Sep 11, 2008 at 03:29:55AM -0700, Dan Price wrote:
> http://cr.opensolaris.org/~dp/ips-fmri-rework.2
client.py:
- line 170: move down to 201
- line 257, 260: error(), like on 252?
- line 269, 343, 1018: emsg()?
- line 277: this loop will have "pkg:" repeatedly. Perhaps do something
more like the way display_catalog_failures() handles multiple failures?
- line 1488: just delete the line
depend.py:
- line 65: why not just set correction here, rather than in clean_fmri?
- line 119: I guess I don't understand why this isn't converting to int()
and then back to str().
fmri.py:
- line 48: just inherit from Exception, and drop the import.
transaction.py:
- the caller of open() (repository.py:open_0) expects return values
currently. You need to coordinate that function with your changes
here.
- line 93: I don't think this is the right reason. The IndexError comes
from tokens[0] failing. I'm not quite sure what tokens is here, but
the error message should reference that. And maybe take the
urllib.quote() thing out of the try block so that it's not as
confusing.
- line 106: This is way too much. The "reason phrase" needs to be short,
and be a single line, which won't always be the case here. I would
simply say "Illegal FMRI" for the reason here. Put more text into the
body of the response if you really need to send it back. We can make
that part of the protocol, if we need the client to have that
information.
- line 111: truncate to "Missing Version"
- line 119: drop the period
version.py:
- line 56: in the spirit of renaming variables that hide module and type
names, "str" is a type name.
- line 57: this also throws if it just can't be converted (like "fred",
or an InvalidFMRIException or whatever).
- line 74: Capitalize "DotSequence" in string
- line 128: second half of what?
t_version.py:
- line 203: "list(v)" instead of comprehension.
t_info_contents.py:
- line 158, 159: are the "exit=0" bits necessary?
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
