Opened 8 years ago

Closed 8 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: master
Severity: Keywords: comments cleanup
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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@…> 8 years ago.

Download all attachments as: .zip

Change History (8)

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

comment:1 Changed 8 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:2 Changed 8 years ago by gwilson

  • Resolution set to wontfix
  • Status changed from new to closed

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

  • Has patch unset
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Summary changed from Cleanup of django/contrib/comments/feeds.py to Mixed up comments in django/contrib/comments/feeds.py
  • Triage Stage changed from Ready for checkin to 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 Changed 8 years ago by Ludvig Ericson <ludvig.ericson@…>

  • Summary changed from Mixed up comments in django/contrib/comments/feeds.py to Mixed up docs in django/contrib/comments/feeds.py

Maybe docs is a better word.

comment:5 Changed 8 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from reopened to 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 Changed 8 years ago by Ludvig Ericson <ludvig.ericson@…>

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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

Read the contents, they're plain wrong.

comment:7 Changed 8 years ago by Ludvig Ericson <ludvig.ericson@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

It has been changed.

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