Message49927
Logged In: YES
user_id=261020
Some mostly-stylistic / minor comments on the patch from a
quick skim (I hope to post some comments on the trickier
issues later):
Follow PEP 8. Some issues I noticed:
- Inconsistent use of case: URI vs. Uri.
- Triple-quoted docstrings should use " not ' for
editor-friendliness.
- Strings should not be abused as comments: If you mean to
use a docstring, use a docstring; otherwise, use a comment
(I'm referring here to your use of strings immediately
*before* def statements).
- import usage like import posixpath as ppath is usually
frowned upon: just import posixpath.
- Use of whitespace in e.g. dict displays and listcomps is
non-standard. [x for x in y], not [ x for x in y ]
- Indentation in docstrings is non-standard.
- Docstring-writing conventions are non-standard.
Other things:
- Having read your original python-dev post, I still think
UrlParser / URIParser could be simpler. I'll try and supply
an actual suggested patch later.
- MailToURIParser appears to support a different interface
to all the others. If this is really necessary for
standards or pragmatic reasons, those parse and unparse
methods should just be separate functions.
- Documentation for the module is missing. This would
document the API and perhaps briefly explain the background
(what's changed to require this new module) and correct
usage, briefly explaining terms like "URI reference". Some
well-chosen examples are always good, of course.
- The tests should go in a separate module
test/test_<modulename>.py and follow the conventions there.
- Would be very nice to explicitly reference RFC 3986
section numbers in the code. I'll try and do this when I
review it properly.
- Use of URI vs. URL distinction is incorrect.
Finally, just BTW:
http://en.wikipedia.org/wiki/Uniform_Resource_Identifier
"""
The contemporary point of view among the working group that
oversees URIs is that the terms URL and URN are
context-dependent aspects of URI and rarely need to be
distinguished.
"""
Heh, spot on! Still, like I said, I agree terms like "URI
reference" deserve to be adopted.
|
|
Date |
User |
Action |
Args |
2007-08-23 15:47:32 | admin | link | issue1462525 messages |
2007-08-23 15:47:32 | admin | create | |
|