Issue 2827: [issue3300] urllib.quote and unquote Code Review (original) (raw)
Description
http://bugs.python.org/issue3300
Patch Set 1#
Patch Set 2 : parse.py.patch8#
Patch Set 3 : patch 9#
Patch Set 4 : patch 10#
Messages
Total messages: 17
| GvR | Hi Matt, Here's a code review of your patch. I'm leaning more and more towards ... | 16 years, 9 months ago (2008-08-06 21:39:34 UTC)#1 |
|---|
| mgiuca | Hi Guido, Thanks very much for this very detailed review. I've replied to the comments. ... | 16 years, 9 months ago (2008-08-07 12:01:31 UTC)#2 |
|---|
| Antoine Pitrou | http://codereview.appspot.com/2827/diff/1/4 File Lib/email/utils.py (right): http://codereview.appspot.com/2827/diff/1/4#newcode224 Line 224: except: On 2008/08/07 12:01:31, mgiuca wrote: > But ... | 16 years, 9 months ago (2008-08-07 12:49:11 UTC)#3 |
|---|
| GvR | | 16 years, 9 months ago (2008-08-07 16:58:56 UTC)#4 | | ------- | | ----------------------------------------------------------- |
| Antoine Pitrou | http://codereview.appspot.com/2827/diff/17/28 File Lib/urllib/parse.py (right): http://codereview.appspot.com/2827/diff/17/28#newcode275 Line 275: except KeyError: If this is supposed to catch ... | 16 years, 9 months ago (2008-08-07 17:29:40 UTC)#5 |
|---|
| GvR | New code review. Also please address Antoine's comments. http://codereview.appspot.com/2827/diff/1/2 File Doc/library/urllib.parse.rst (right): http://codereview.appspot.com/2827/diff/1/2#newcode198 Line 198: ... | 16 years, 9 months ago (2008-08-07 17:38:01 UTC)#6 |
|---|
| mgiuca | Oh dear, it seems I never published my comments. I made all of these before ... | 16 years, 9 months ago (2008-08-10 05:39:34 UTC)#7 |
|---|
| mgiuca | http://codereview.appspot.com/2827/diff/17/28 File Lib/urllib/parse.py (right): http://codereview.appspot.com/2827/diff/17/28#newcode275 Line 275: except KeyError: Good catch. No test case caught ... | 16 years, 9 months ago (2008-08-10 05:53:52 UTC)#8 |
|---|
| mgiuca | http://codereview.appspot.com/2827/diff/1/2 File Doc/library/urllib.parse.rst (right): http://codereview.appspot.com/2827/diff/1/2#newcode223 Line 223: Replace ``%xx`` escapes by their single-character equivalent. I ... | 16 years, 9 months ago (2008-08-10 07:06:56 UTC)#9 |
|---|
| mgiuca | http://codereview.appspot.com/2827/diff/17/28 File Lib/urllib/parse.py (right): http://codereview.appspot.com/2827/diff/17/28#newcode275 Line 275: except KeyError: On 2008/08/10 05:53:52, mgiuca wrote: > ... | 16 years, 9 months ago (2008-08-10 07:45:01 UTC)#10 |
|---|
| GvR | | 16 years, 9 months ago (2008-08-12 18:53:45 UTC)#11 | | ------- | | ------------------------------------------------------------- |
| GvR | Here's finally my code review. Most stuff was already discussed in the tracker however. http://codereview.appspot.com/2827/diff/82/83 ... | 16 years, 9 months ago (2008-08-13 17:50:13 UTC)#12 |
|---|
| GvR | BTW, I also had a look at Bill's patch. The one good idea I hope ... | 16 years, 9 months ago (2008-08-13 18:48:33 UTC)#13 |
|---|
| mgiuca | Hi guys, thanks for these detailed reviews. Here are my comments to accompany patch 10. ... | 16 years, 9 months ago (2008-08-14 15:28:59 UTC)#14 |
|---|
| GvR | | 16 years, 9 months ago (2008-08-18 18:37:10 UTC)#15 | | ------- | | ------------------------------------------------------------- |
| GvR | Hey Matt, I have a bunch of tiny nits, but there's no need for you ... | 16 years, 9 months ago (2008-08-18 20:20:32 UTC)#16 |
|---|
| GvR | Submitted as r65838, with these changes. http://codereview.appspot.com/2827/diff/79/103 File Lib/urllib/parse.py (right): http://codereview.appspot.com/2827/diff/79/103#newcode277 Line 277: if item ... | 16 years, 9 months ago (2008-08-18 21:45:42 UTC)#17 |
|---|