Opened 17 years ago
Closed 17 years ago
#5766 closed (fixed)
Mixed up docs in django/contrib/comments/feeds.py
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Contrib apps | Version: | dev |
Severity: | Keywords: | comments cleanup | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
django/contrib/comments/feeds.py has a hard-wired limit, and the comments are mixed up, and in different styles.
Patch lets one do:
class MyCommentsFeed(LatestFreeCommentsFeed): comment_count_limit = 10
Attachments (1)
Change History (8)
by , 17 years ago
Attachment: | comments-cleanup-r6522.diff added |
---|
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:2 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:3 by , 17 years ago
Has patch: | unset |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
Summary: | Cleanup of django/contrib/comments/feeds.py → Mixed up comments in django/contrib/comments/feeds.py |
Triage Stage: | Ready for checkin → Unreviewed |
Yeah, but the feeds framework calls those automatically, I don't - but fine, there's no point in arguing about that, just that my way is the easier fix for a larger problem.
Notice how there's still a bug, with the comments of both classes. They're plain wrong.
comment:4 by , 17 years ago
Summary: | Mixed up comments in django/contrib/comments/feeds.py → Mixed up docs in django/contrib/comments/feeds.py |
---|
Maybe docs is a better word.
comment:5 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
I agree with Gary -- no need to change the functionality here. The comments aren't wrong either: a docstring can be any string and there are strings like this as docstrings throughout Django.
comment:6 by , 17 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
I don't care what string notation you use, not the least bit.
Read the contents, they're plain wrong.
Well, you could just use the queryset directly instead of the
items()
convenience method:As far as the patch goes, I'm not sure if this should be a class attribute. Maybe just a parameter to
items
:But seeing how any of this would just be another way to do something that's already possible, I'm going to close this as a wontfix.