Mailing List Archive

[Bug 7826] Improve language around whitelist/blacklist and master/slave
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

Kevin A. McGrail <kmcgrail@apache.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |kmcgrail@apache.org

--- Comment #5 from Kevin A. McGrail <kmcgrail@apache.org> ---
First test is done with allowlist_to replacing whitelist_to
Committed revision 1879456.

NOTE: Not sure how active.list for rules will handle the new rules that are
version based for this but the goal is that rules will work on older versions
and 4.0.0 has an alias for whitelist_to so both configuration lines are
equivalent.

Based on this Proof of Concept, will continue working on more changes.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

Henrik Krohns <apache@hege.li> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |apache@hege.li

--- Comment #6 from Henrik Krohns <apache@hege.li> ---
Kevin you gonna break everyone running old trunk versions with the eval
function name changes.

We should not check for "version 4.0.0", instead always use has_feature_xxx.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #7 from Henrik Krohns <apache@hege.li> ---
Atleast leave the old eval functions as compatibility layer. Just call the new
ones from the old.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #8 from Kevin A. McGrail <kmcgrail@apache.org> ---
Henrik,

I considered that but with 4.0 unreleased, there is an expectation that trunk
will be bleeding edge. I want the future move to remove the backwards
compatibility to be simple.

And since plugin WLBLEval will be changing to ALBLEval.pm, there is a level of
breakage that is coming from this change with 4.0 that has_xxx and leaving old
routines won't help.

I should give a heads-up to the list though for people running trunk that they
might want to keep an eye on things on list.

KAM

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #9 from AXB <axb.lists@gmail.com> ---
did this motion get the required PCM votes?

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #10 from AXB <axb.lists@gmail.com> ---
(In reply to AXB from comment #9)
> did this motion get the required PCM votes?

I mean PMC

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #11 from Henrik Krohns <apache@hege.li> ---
(In reply to AXB from comment #9)
> did this motion get the required PCM votes?

Yeah though atleast few votes were conditional on backwards compatibility..

I haven't seen any discussion agreeing on changing module names. Are we going
to replace everything including autoWHITELIST, WHITELISTsubject, etc.. atleast
do NOT commit these as random single commits over time. Do _all_ changes in a
single commit so there's atleast some consistent revision where compatibility
is broken.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #12 from Henrik Krohns <apache@hege.li> ---
Also care should be taken if renaming modules, as new "make install" will just
leave the old ones lingering. Also nothing will change the old *.pre files to
use the new modules! It requires careful planning to not break anything.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #13 from AXB <axb.lists@gmail.com> ---

" The PMC has voted to support changing whitelist to allowlist and blacklist to
blocklist as well as master to [arent and slave will be changed to child."

Whre are all those votes?


Without keeping backward compatibilty for a decade, these changes should be
immediately reversed.

Nobody can predict what the outcome will be if changes are done in batches.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #14 from AXB <axb.lists@gmail.com> ---
-1

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #15 from Henrik Krohns <apache@hege.li> ---
I'd recommend a separate branch to test things.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #16 from AXB <axb.lists@gmail.com> ---
(In reply to Henrik Krohns from comment #15)
> I'd recommend a separate branch to test things.

I can relate to that.
Also, plans for such changes should be transparently shared with the SA user's
list as these will be the ones affected.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #17 from AXB <axb.lists@gmail.com> ---
(In reply to Henrik Krohns from comment #6)
> Kevin you gonna break everyone running old trunk versions with the eval
> function name changes.
>
> We should not check for "version 4.0.0", instead always use has_feature_xxx.

don't masscheckers use trunk?

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #18 from Henrik Krohns <apache@hege.li> ---
(In reply to AXB from comment #17)
>
> don't masscheckers use trunk?

masscheckers are supposed to automatically download latest revision matching
the rules.. (atleast automasscheck script handles this)

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

macosx10.4.11@gmail.com changed:

What |Removed |Added
----------------------------------------------------------------------------
CC|macosx10.4.11@gmail.com |

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #19 from Henrik Krohns <apache@hege.li> ---
Seems atleast last masscheck broke

Jul 5 04:40:24.987 [18525] warn: rules: failed to run __FROM_ADDRLIST_GOV
test, skipping:
Jul 5 04:40:24.987 [18525] warn: \t(Can't locate object method
"_check_whitelist" via package "Mail: [...]:SpamAssassin::Plugin::WLBLEval" at
../lib/Mail/SpamAssassin/Plugin/WLBLEval.pm line 119.
Jul 5 04:40:24.987 [18525] warn: )

So how bout we rollback the patch and start with a branch?

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #20 from AXB <axb.lists@gmail.com> ---
(In reply to Henrik Krohns from comment #19)
> Seems atleast last masscheck broke

> So how bout we rollback the patch and start with a branch?

good early morning &

+1

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

John Hardin <jhardin@impsec.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |jhardin@impsec.org

--- Comment #21 from John Hardin <jhardin@impsec.org> ---
(In reply to Henrik Krohns from comment #19)

> So how bout we rollback the patch and start with a branch?

+1

> Jul 5 04:40:24.987 [18525] warn: \t(Can't locate object method
> "_check_whitelist" via package "Mail: [...]:SpamAssassin::Plugin::WLBLEval"
> at ../lib/Mail/SpamAssassin/Plugin/WLBLEval.pm line 119.

Also, re terminology: we can retain "W" (as in "WLBLEval") if we adopt "W" =
"Welcome": WelcomeList/BlockList.

That might help avoid some of the renaming.

Also, the internal method names should be retained for backwards compatibility.
They are not generally visible to non-developers and may be in use by
third-party plugins.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #22 from Kevin A. McGrail <kmcgrail@apache.org> ---
Hi All,

Since trunk is an unreleased branch, I am -1 to to switch to another branch.
Users will be carefully alerted via an UPGRADE file and release announcements
for 4.0.

We did one patch to the system to prove it worked before we went through all
the various functions one by one. I wanted the patch to be bite size and
reviewable.

To fix masscheck, a temporary stub for backwards compatibility until all the
functions are done should work like this:

sub _check_whitelist {
return _check_allowlist(@_);
}

I like the idea of welcome and block to replace white and black. I +1 that and
can revert this change, switch from allowlist to welcomelist and add the stub
if others concur.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #23 from Bill Cole <billcole@apache.org> ---
(In reply to Kevin A. McGrail from comment #22)

> I like the idea of welcome and block to replace white and black. I +1 that
> and can revert this change, switch from allowlist to welcomelist and add the
> stub if others concur.

+1 for the acronym-preserving nomenclature.

+1 for moving forward on trunk with stubs as needed. The only way to get any
exercise of new code is by putting it on trunk, as virtually no one will run
the head of an explicit test branch.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #24 from AXB <axb.lists@gmail.com> ---
(In reply to Bill Cole from comment #23)
> (In reply to Kevin A. McGrail from comment #22)
>
> > I like the idea of welcome and block to replace white and black. I +1 that
> > and can revert this change, switch from allowlist to welcomelist and add the
> > stub if others concur.
>
> +1 for the acronym-preserving nomenclature.
>
> +1 for moving forward on trunk with stubs as needed. The only way to get any
> exercise of new code is by putting it on trunk, as virtually no one will run
> the head of an explicit test branch.

As I don't understand the need for all these changes I will abstain from any
further votes.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #25 from Henrik Krohns <apache@hege.li> ---
(In reply to Bill Cole from comment #23)
>
> +1 for moving forward on trunk with stubs as needed. The only way to get any
> exercise of new code is by putting it on trunk, as virtually no one will run
> the head of an explicit test branch.

Branch will allow testing out the changes safely. Alternatively, don't commit
unfinished and untested tryouts then. Trunk as a mass check branch, is not a
test bed for these kinds of invasive changes.

If Kevin is doing this alone and disappearing for days every time masschecks
and things break, it's not looking good for the project.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

--- Comment #26 from Henrik Krohns <apache@hege.li> ---
(In reply to Kevin A. McGrail from comment #22)
>
> We did one patch to the system to prove it worked before we went through all
> the various functions one by one. I wanted the patch to be bite size and
> reviewable.

Who is "we"? It was obvious within seconds from looking at the commit that it's
going to break things. If you don't have time to properly review things
yourself, commit a branch or post a patch, so others can review it without
breaking things.

> To fix masscheck, a temporary stub for backwards compatibility until all the
> functions are done should work like this:
>
> sub _check_whitelist {
> return _check_allowlist(@_);
> }

The backwards compatibility should be for check_to_in_whitelist eval function,
not _check_whitelist. And why should it be temporary?? Just leave it there!
People can have local rules too which sa-update has no control over.

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

Giovanni Bechis <giovanni@paclan.it> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |giovanni@paclan.it

--- Comment #27 from Giovanni Bechis <giovanni@paclan.it> ---
Created attachment 5709
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5709&action=edit
Unbreak allowlist

Quick fix to unbreak masscheck, more fixes needed to have good-looking code.
I think backwards compatibility should be a must due to custom rules that could
depend on changed eval().

--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7826] Improve language around whitelist/blacklist and master/slave [ In reply to ]
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7826

Loren Wilton <lwilton@earthlink.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |lwilton@earthlink.net

--- Comment #28 from Loren Wilton <lwilton@earthlink.net> ---
Personally I consider this whole concept to be extremely ill-advised and
strongly suggest that it be dropped.

Has anyone ever considered bringing this change up on the SA users list and
discussing it there? It seems that users should have some say in a matter that
will affect most all of them, such as forcing virtually everyone with local
rules to rewrite their rules. I can find no reference to it in the SA users
list.

It's all well and good for some shadowy very small group of people to make
overall policy decisions that will affect many without soliciting input from
the people affected. But it isn't always the wisest choice.

--
You are receiving this mail because:
You are the assignee for the bug.

1 2 3  View All