Opened 16 years ago

Closed 16 years ago

#5766 closed (fixed)

Mixed up docs in django/contrib/comments/feeds.py

Reported by: Ludvig Ericson <ludvig.ericson@…> 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)

comments-cleanup-r6522.diff (1005 bytes ) - added by Ludvig Ericson <ludvig.ericson@…> 16 years ago.

Download all attachments as: .zip

Change History (8)

by Ludvig Ericson <ludvig.ericson@…>, 16 years ago

Attachment: comments-cleanup-r6522.diff added

comment:1 by Chris Beaven, 16 years ago

Triage Stage: UnreviewedReady for checkin

comment:2 by Gary Wilson, 16 years ago

Resolution: wontfix
Status: newclosed

Well, you could just use the queryset directly instead of the items() convenience method:

myfeed = LatestFreeCommentsFeed(...)
myfeed.get_query_set()[:10]

As far as the patch goes, I'm not sure if this should be a class attribute. Maybe just a parameter to items:

    def items(self, limit=40):
        if limit:
            return self.get_query_set()[:limit]
        else:
            return self.get_query_set()

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.

comment:3 by Ludvig Ericson <ludvig.ericson@…>, 16 years ago

Has patch: unset
Resolution: wontfix
Status: closedreopened
Summary: Cleanup of django/contrib/comments/feeds.pyMixed up comments in django/contrib/comments/feeds.py
Triage Stage: Ready for checkinUnreviewed

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 Ludvig Ericson <ludvig.ericson@…>, 16 years ago

Summary: Mixed up comments in django/contrib/comments/feeds.pyMixed up docs in django/contrib/comments/feeds.py

Maybe docs is a better word.

comment:5 by Malcolm Tredinnick, 16 years ago

Resolution: wontfix
Status: reopenedclosed

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 Ludvig Ericson <ludvig.ericson@…>, 16 years ago

Resolution: wontfix
Status: closedreopened

I don't care what string notation you use, not the least bit.

Read the contents, they're plain wrong.

comment:7 by Ludvig Ericson <ludvig.ericson@…>, 16 years ago

Resolution: fixed
Status: reopenedclosed

It has been changed.

Note: See TracTickets for help on using tickets.
Back to Top