Opened 11 years ago

Closed 11 years ago

#5766 closed (fixed)

Mixed up docs in django/contrib/comments/

Reported by: Ludvig Ericson <ludvig.ericson@…> Owned by: nobody
Component: Contrib apps Version: master
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


django/contrib/comments/ 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@…> 11 years ago.

Download all attachments as: .zip

Change History (8)

Changed 11 years ago by Ludvig Ericson <ludvig.ericson@…>

Attachment: comments-cleanup-r6522.diff added

comment:1 Changed 11 years ago by Chris Beaven

Triage Stage: UnreviewedReady for checkin

comment:2 Changed 11 years ago by Gary Wilson

Resolution: wontfix
Status: newclosed

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

myfeed = LatestFreeCommentsFeed(...)

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]
            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 Changed 11 years ago by Ludvig Ericson <ludvig.ericson@…>

Has patch: unset
Resolution: wontfix
Status: closedreopened
Summary: Cleanup of django/contrib/comments/feeds.pyMixed up comments in django/contrib/comments/
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 Changed 11 years ago by Ludvig Ericson <ludvig.ericson@…>

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

Maybe docs is a better word.

comment:5 Changed 11 years ago by Malcolm Tredinnick

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 Changed 11 years ago by Ludvig Ericson <ludvig.ericson@…>

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 Changed 11 years ago by Ludvig Ericson <ludvig.ericson@…>

Resolution: fixed
Status: reopenedclosed

It has been changed.

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