Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

misleading comment in urllib2 #48746

Closed
jjlee mannequin opened this issue Dec 2, 2008 · 4 comments
Closed

misleading comment in urllib2 #48746

jjlee mannequin opened this issue Dec 2, 2008 · 4 comments
Labels
stdlib Python modules in the Lib dir

Comments

@jjlee
Copy link
Mannequin

jjlee mannequin commented Dec 2, 2008

BPO 4496
Nosy @facundobatista, @amauryfa, @orsenthil, @bitdancer

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2010-12-23.19:52:59.206>
created_at = <Date 2008-12-02.21:07:38.625>
labels = ['library']
title = 'misleading comment in urllib2'
updated_at = <Date 2010-12-24.02:46:00.105>
user = 'https://bugs.python.org/jjlee'

bugs.python.org fields:

activity = <Date 2010-12-24.02:46:00.105>
actor = 'orsenthil'
assignee = 'none'
closed = True
closed_date = <Date 2010-12-23.19:52:59.206>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2008-12-02.21:07:38.625>
creator = 'jjlee'
dependencies = []
files = []
hgrepos = []
issue_num = 4496
keywords = []
message_count = 4.0
messages = ['76783', '76801', '124567', '124584']
nosy_count = 6.0
nosy_names = ['jhylton', 'facundobatista', 'jjlee', 'amaury.forgeotdarc', 'orsenthil', 'r.david.murray']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue4496'
versions = ['Python 3.0', 'Python 2.7']
@jjlee
Copy link
Mannequin Author

jjlee mannequin commented Dec 2, 2008

r54137 replaced a comment in urllib2.py with a misleading comment. The
comment now implies the .insort() in question achieves the goal
specified in the new comment. That's not true, which was the reason the
original comment was there in the first place.

At least replace the comment with the original comment. Alternatively,
(correctly) resolve the question in the original comment somehow (for
example by removing the .handlers attribute -- but since .handlers is
misnamed in not having an initial underscore, it may have tempted people
to use it, even though it's undocumented, hence private).

@jjlee jjlee mannequin added the stdlib Python modules in the Lib dir label Dec 2, 2008
@amauryfa
Copy link
Member

amauryfa commented Dec 2, 2008

(The change was actually r54138)
It seems that this .handlers attribute was used in older versions,
before r34922, to close the handlers in-order.
IMO it is no more useful, but may be kept for compatibility reasons.

@bitdancer
Copy link
Member

It is used in the tests, but I agree that it doesn't appear to be used in the code. I've removed the misleading comment and marked the self.handlers attribute as backward-compat-only in r87448, r87449, and r87450.

The sorting is based on a 'handler_order' attribute, by the way, and presumably does control the order in which they are applied.

@orsenthil
Copy link
Member

On Thu, Dec 23, 2010 at 07:53:01PM +0000, R. David Murray wrote:

The sorting is based on a 'handler_order' attribute, by the way, and
presumably does control the order in which they are applied.

Yes. Exactly.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stdlib Python modules in the Lib dir
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@orsenthil @bitdancer @amauryfa