Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2)

Issue 3255: Issue #3602 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 3 months ago by Benjamin
Modified:
3 months, 3 weeks ago
Reviewers:
brett.cannon
CC:
SVN Base:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Description

Brett's patch from #3602.

Patch Set 1

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/BaseHTTPServer.py View 1 chunk 21 lines 0 comments Download
Lib/asynchat.py View 2 chunks 27 lines 2 comments Download
Lib/cgi.py View 1 chunk 25 lines 0 comments Download
Lib/httplib.py View 1 chunk 22 lines 0 comments Download
Lib/mimetools.py View 1 chunk 21 lines 0 comments Download
Lib/test/test_support.py View 2 chunks 88 lines 2 comments Download
Lib/test/test_warnings.py View 11 chunks 202 lines 0 comments Download
Lib/warnings.py View 2 chunks 97 lines 9 comments Download
Python/_warnings.c View 2 chunks 31 lines 0 comments Download

Messages

Total messages: 2
Benjamin
I'm not sure why you combined the deprecation warning fix patch with this, but it ...
1 year, 3 months ago
brett.cannon
1 year, 3 months ago
http://codereview.appspot.com/3255/diff/1/3
File Lib/asynchat.py (right):

http://codereview.appspot.com/3255/diff/1/3#newcode52
Line 52: from sys import py3kwarning
On 2008/08/23 01:46:36, Benjamin wrote:
> I think it's better to just "import sys" here, so we don't have a bunch of
> modules with "py3kwarning" in them sitting around.

Maybe, but you could say this about any import. I don't see it as a problem.

http://codereview.appspot.com/3255/diff/1/10
File Lib/test/test_support.py (right):

http://codereview.appspot.com/3255/diff/1/10#newcode385
Line 385: return warnings.catch_warnings(record=record, module=module)
On 2008/08/23 01:46:36, Benjamin wrote:
> Why not just do "from warnings import catch_warnings"?

There are other calls to 'warnings' in the module.

http://codereview.appspot.com/3255/diff/1/4
File Lib/warnings.py (right):

http://codereview.appspot.com/3255/diff/1/4#newcode287
Line 287: class WarningMessage(object):
On 2008/08/23 01:46:36, Benjamin wrote:
> What happened to the idea of making this a namedtuple?

Can't import _collections in a svn checkout since 'warnings' is imported before
site.

http://codereview.appspot.com/3255/diff/1/4#newcode320
Line 320: while True:
On 2008/08/23 01:46:36, Benjamin wrote:
> Can't this be "del self[:]"?

Yep.

http://codereview.appspot.com/3255/diff/1/4#newcode327
Line 327: class catch_warnings(object):
On 2008/08/23 01:46:36, Benjamin wrote:
> Shouldn't this be called "catchwarnings"?

It's a toss-up thanks to warn_explicit(). I prefer underscores, so I just stuck
with it.

http://codereview.appspot.com/3255/diff/1/4#newcode327
Line 327: class catch_warnings(object):
On 2008/08/23 01:46:36, Benjamin wrote:
> Can't this be a simple generator contextmanager?

No because _collections can't be imported.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld r496