django.contrib.comments is judging me
Owen Nelson
Owen Nelson
Adrian Holovaty
> Moving forward, I'll be skipping the comment-specific setting and simply
> setting PROFANITIES_LIST to (). Any chance we'll see the setting removed in
> the near future?
Yes, it's about time we got rid of this $&*@ thing!
(That was too easy.)
I don't see any great reason to keep this in, because it was never
documented and more importantly, as Owen points out, it's not the
responsibility of a Web framework. If nobody has any rational reasons
to leave it in, I can remove it. I'll wait a day or so for folks to
chime in with reasons to keep it.
Adrian
Russell Keith-Magee
Strictly, it needs to be put on a deprecation path, because it *is*
documented, in ref/settings.txt. So the earliest we can truly remove
it is in 1.5, after a PendingDeprecationWarning in 1.3 and a full
Warning in 1.4.
Along the way, we should also be removing COMMENTS_ALLOW_PROFANITIES,
since there isn't much point having one without t'other.
However, other than that: +1.
Yours,
Russ Magee %-)
Owen Nelson
Strictly, it needs to be put on a deprecation path, because it *is*
documented, in ref/settings.txt. So the earliest we can truly remove
it is in 1.5, after a PendingDeprecationWarning in 1.3 and a full
Warning in 1.4.
Russell Keith-Magee
As luck would have it, there is already a ticket: #8794. I've just
marked it as a 1.3 milestone task.
Yours,
Russ Magee %-)
Owen Nelson
Luke Plant
> Strictly, it needs to be put on a deprecation path, because it *is*
> documented, in ref/settings.txt. So the earliest we can truly remove
> it is in 1.5, after a PendingDeprecationWarning in 1.3 and a full
> Warning in 1.4.
>
> Along the way, we should also be removing COMMENTS_ALLOW_PROFANITIES,
> since there isn't much point having one without t'other.
Although we cannot remove it in 1.3, could we set the default value to
an empty list? I imagine that people who are depending on its
functionality will have set their own value in their settings.py, and
they can easily do so if they haven't already.
Luke
--
"Dysfunction: The only consistent feature of all of your
dissatisfying relationships is you." (despair.com)
Luke Plant || http://lukeplant.me.uk/
Russell Keith-Magee
> On Thu, 2010-10-07 at 11:30 +0800, Russell Keith-Magee wrote:
>
>> Strictly, it needs to be put on a deprecation path, because it *is*
>> documented, in ref/settings.txt. So the earliest we can truly remove
>> it is in 1.5, after a PendingDeprecationWarning in 1.3 and a full
>> Warning in 1.4.
>>
>> Along the way, we should also be removing COMMENTS_ALLOW_PROFANITIES,
>> since there isn't much point having one without t'other.
>
> Although we cannot remove it in 1.3, could we set the default value to
> an empty list? I imagine that people who are depending on its
> functionality will have set their own value in their settings.py, and
> they can easily do so if they haven't already.
The set of people that will be affected here are are all opt-in --
they've had to set COMMENTS_ALLOW_PROFANITIES=True -- so I would have
assumed that some subset will be happy with the default
PROFANITIES_LIST as currently defined. The behavior in question is
pretty useless, but it is a known quantity; changing the default
behavior while we're also deprecating just sounds like a source of
confusion to me.
Yours,
Russ Magee %-)
Luke Plant
> The set of people that will be affected here are are all opt-in --
> they've had to set COMMENTS_ALLOW_PROFANITIES=True -- so I would have
> assumed that some subset will be happy with the default
> PROFANITIES_LIST as currently defined. The behavior in question is
> pretty useless, but it is a known quantity; changing the default
> behavior while we're also deprecating just sounds like a source of
> confusion to me.
? AFAICS, the filter is on by default, so everyone who uses
contrib.comments is affected unless they've opted *out* - resulting in
the surprise shown by Owen at the beginning of this thread.
If we don't change the default behaviour, it might be worth fixing the
Scunthorpe problem [1] - it's a genuine bug in it's own right, and it
can catch people who haven't enabled opted in to anything. Patch for
that attached.
Luke
[1] http://code.djangoproject.com/ticket/8794
Russell Keith-Magee
> On Thu, 2010-10-07 at 18:50 +0800, Russell Keith-Magee wrote:
>
>> The set of people that will be affected here are are all opt-in --
>> they've had to set COMMENTS_ALLOW_PROFANITIES=True -- so I would have
>> assumed that some subset will be happy with the default
>> PROFANITIES_LIST as currently defined. The behavior in question is
>> pretty useless, but it is a known quantity; changing the default
>> behavior while we're also deprecating just sounds like a source of
>> confusion to me.
>
> ? AFAICS, the filter is on by default, so everyone who uses
> contrib.comments is affected unless they've opted *out* - resulting in
> the surprise shown by Owen at the beginning of this thread.
>
> If we don't change the default behaviour, it might be worth fixing the
> Scunthorpe problem [1] - it's a genuine bug in it's own right, and it
> can catch people who haven't enabled opted in to anything. Patch for
> that attached.
My apologies, you're right -- I had the logic reversed in my head. I'm
not having a good day.
Clearing the PROFANITIES_LIST could be considered a "fix" for
Scunthorpe, but it's a bit extreme given that it is the existing
behavior (and again, people *could* be relying on it). I'm inclined to
say that your improved regex approach is a better option for the short
term, along with deprecation for the long term.
Yours,
Russ Magee %-)
Jacob Kaplan-Moss
<rus...@keith-magee.com> wrote:
> Strictly, it needs to be put on a deprecation path [...]
You know, I really don't think this is a big enough deal to bother
being completely picky about the deprecation policy here. It's a silly
setting that should have been removed pre-open-source when we did the
whole Django/Ellington split.
I'm +1 on Luke's idea: set it to the empty list in 1.3, then remove it
entirely in 1.4.
Yes, it doesn't exactly follow our normal process. No, I don't
particularly care.
Jacob
Adrian Holovaty
> You know, I really don't think this is a big enough deal to bother
> being completely picky about the deprecation policy here. It's a silly
> setting that should have been removed pre-open-source when we did the
> whole Django/Ellington split.
>
> I'm +1 on Luke's idea: set it to the empty list in 1.3, then remove it
> entirely in 1.4.
>
> Yes, it doesn't exactly follow our normal process. No, I don't
> particularly care.
Thank you for the pragmatism, Jacob. It saved me from going on a
public rant about overly cautious backwards-compatibility policies.
Yes, it's a silly setting that should have been removed circa 2006.
Anybody with mission-critical systems relying on it deserves to be
branded with the profanities contained therein.
I've set it to an empty tuple in http://code.djangoproject.com/changeset/13996 .
As a next step, I'd like to remove PROFANITIES_LIST and
COMMENTS_ALLOW_PROFANITIES from global_settings.py entirely, moving
the default values directly into django.contrib.comments. (I.e., the
django.contrib.comments code will check for the existence of a
setting, use it if it exists, and fall back on hard-coded defaults
otherwise.)
Adrian