Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#13199 closed (invalid)

Better documentation of things to avoid in the comments framework

Reported by: Yeago Owned by: nobody
Component: contrib.comments Version: 1.2-beta
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I got myself into a situation with a project and some comments models. Perhaps with some discussion we can uncover genuine code-bugs, but my knowledge about this is limited and so I'm just calling for a documentation update. A major reason I'm asking for some notice of these things to appear somewhere is that all of the below seems to work perfectly fine in the dev server. On deployment, its a confusing catastrophy that, at best, gives you a vague ImproperlyConfigured("The COMMENTS_APP setting refers to a non-existing package."), which is of course no help at all.

Three things:

1) Its not obvious to me why, but importing a custom comment model as 'Comment' seems, somehow, to get back upstream to django.contrib.comments and wreck things. If this is due to the nature of that particular frameworks' immaculate ability to be extensive and provide conveniences all around, so be it. I'd just like a notice next time before I attempt to save code by doing Comment = comments.get_model() and expect everything to be honkey dorey.

2) A custom comments app should not also be called 'comments'. Probably a very obvious one but the problems it creates are inconsistent between dev and live and not obvious.

3) Models hoping to foreignkey to whatever comments model the app provides should not be passed the callable. I realize passing callables into a ForeignKey probably seems strange in and of itself, however, once again it creates inconsistent problems between dev and live which never connect back to the FK in any obvious way.

Change History (10)

comment:2 by Yeago, 14 years ago

Another nag is that the django.contrib.comments.get_model() callable won't be respected if you use it as a signal sender.

comment:3 by Yeago, 14 years ago

milestone: 1.2
Version: 1.11.2-beta

comment:4 by James Bennett, 14 years ago

milestone: 1.2

When considering whether something belongs on the 1.2 milestone, there are three questions which matter:

  1. Is this something which worked in Django 1.1 but does not work in Django 1.2?
  2. Is this a bug which prevents a new feature in 1.2 from working?
  3. Does this cause data to be destroyed/deleted?

If the answer to all of those is "no", as it is here, then the ticket doesn't go on 1.2.

comment:5 by Yeago, 14 years ago

If its purely documentation, I didn't really see the harm and I don't mind doing it myself.

comment:6 by Russell Keith-Magee, 14 years ago

Resolution: invalid
Status: newclosed

The key here is *if* it's purely documentation .

If this was clearly a documentation fix, I'd accept it as 'three things that need to be improved in the comments docs'.

However, your own report text indicates that you're not certain that this is just a documentation fix, and I tend to agree. It sounds to me that there might be some bugs hiding in this particular area, if only in the error reporting. The fact that there are three bullet points in your report suggests that there are at least three issues that need to be narrowed down as either bugs or documentation issues.

Trac isn't the place to sort out these issues -- this needs to be a thread on django-developers. Once any discussions there come to resolution, then we can 1) reopen this ticket as an indication of documentation that needs to be improved, and/or 2) submit new tickets for the bugs that need to be fixed.

Either way, this isn't 1.2 critical, for the reasons @ubernostrum mentioned.

comment:7 by Yeago, 14 years ago

Eh, do whatever. Everytime I go there they tell me to open a ticket. If you're just going to close my ticket I don't really care to participate. I spent hours mudging around a nasty wsgi produced log trying to isolate the causes, but I'm sure the next guy will too. Granted it was misflagged as 1.2.

comment:8 by Yeago, 14 years ago

Cc: subsume@… removed

comment:9 by Russell Keith-Magee, 14 years ago

Oh yes - I forgot to mention: Hyperbole is always helpful.

According to my records, you've been involved in 9 threads on django-developers. I can only find one example where you were told to open a ticket, which you did (#12937). In that case, you were reporting a specific problem, with a specific set of models and queries.

In this case, you've reported "There are some things that might be broken, or might be documentation problems". What I've asked you to do is to narrow down *exactly* what problems exist, and that asking django-developers is the best way to do this.

To put it another way - if we left this ticket open, what would be the resolution that closed it? There isn't a clearly identified problem. There isn't a clearly identified solution. There are potentially three different problems, and it's entirely possible we might be able to fix one easily, but a second is difficult to fix. How do we represent that the ticket is "partially fixed" in a way that users searching trac will clearly understand what is a known problem, and what still needs work?

This is a community project, and we need the community to help us resolve problems -- part of which is clearly identifying exactly what problems exist in the first place.

comment:10 by Yeago, 14 years ago

Peachy then.

Nevermind I was speaking collectively about the list and the IRC channel. Also, I've signed up for the dev list under multiple email addresses by accident and which alias appears is a matter of where I am logged in. Indeed, my aliases can be guessed and logs can be parsed for further fruitfulness on that topic.

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