You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2014/01/08 15:46:32 UTC
[Bug 6990] New: Redis.pm fails perlcritic test for package name
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
Bug ID: 6990
Summary: Redis.pm fails perlcritic test for package name
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: Regression Tests
Assignee: dev@spamassassin.apache.org
Reporter: jquinn+SAbug@pccc.com
Created attachment 5178
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=5178&action=edit
error from xt/60_perlcritic.t
See the attached error. Perl::Critic is complaining that the filename below
does not the package name inside the file.
Filename: blib/lib/Mail/SpamAssassin/BayesStore/Redis.pm
Package: Mail::SpamAssassin::BayesStore::TinyRedis
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #5 from Kevin A. McGrail <km...@pccc.com> ---
Yikes, I didn't see that.
Why two packages in the same file? I'm sure you have a good reason for that
which I'm guessing is because you can only specify one bayes_store_module.
But I think we should split to Redis.pm and TinyRedis.pm. So I added a conf of
bayes_store_module_additional and split the files.
You should be able to add this to the conf and everything is like it was but we
also pass the xt/ test
bayes_store_module_additional Mail::SpamAssassin::BayesStore::TinyRedis
svn commit -m 'Split Redis.pm into two files including TinyRedis.pm, added
configuration option to load a second module bug 6990 - and documented
autolearn_force better for bug 5503 as well as documented the multiple and
maxhits tflags better' MANIFEST lib/Mail/SpamAssassin/BayesStore/Redis.pm
lib/Mail/SpamAssassin/Plugin/Bayes.pm lib/Mail/SpamAssassin/Conf.pm
build/announcements/PROPOSED-3.4.0.txt
Sending MANIFEST
Sending build/announcements/PROPOSED-3.4.0.txt
Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
Sending lib/Mail/SpamAssassin/Conf.pm
Sending lib/Mail/SpamAssassin/Plugin/Bayes.pm
Transmitting file data .....
Committed revision 1557309.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #8 from Mark Martinec <Ma...@ijs.si> ---
Jan 11 11:42:44.827 [65959] warn: plugin: eval failed: bayes: Redis failed:
Can't locate object method "new"
via package "Mail::SpamAssassin::BayesStore::TinyRedis"
(perhaps you forgot to load "Mail::SpamAssassin::BayesStore::TinyRedis"?)
at /usr/local/lib/perl5/site_perl/5.18/Mail/SpamAssassin/BayesStore/Redis.pm
line 223.
at /usr/local/lib/perl5/site_perl/5.18/Mail/SpamAssassin/BayesStore/Redis.pm
line 231.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #6 from Kevin A. McGrail <km...@pccc.com> ---
Adding the TinyRedis.pm file of course:
svn commit -m 'Adding TinyRedis.pm'
Adding lib/Mail/SpamAssassin/BayesStore/TinyRedis.pm
Transmitting file data .
Committed revision 1557311.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #10 from Kevin A. McGrail <km...@pccc.com> ---
(In reply to Mark Martinec from comment #9)
> > Can't locate object method "new"
> > via package "Mail::SpamAssassin::BayesStore::TinyRedis"
>
> added a missing "use Mail::SpamAssassin::BayesStore::TinyRedis"
> Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
> Committed revision 1557372.
good call and good point on moving it. Moving it now is the right thing and
I'll build rc5.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #12 from Kevin A. McGrail <km...@pccc.com> ---
svn commit -m 'Add documentation for Redis Bayes backend to Redis.pm, moved
TinyRedis to Util.pm, updated example config for Redis and changed all
TinyRedis references to Util::TinyRedis'
Sending build/announcements/PROPOSED-3.4.0.txt
Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
Deleting lib/Mail/SpamAssassin/BayesStore/TinyRedis.pm
Adding lib/Mail/SpamAssassin/Util/TinyRedis.pm
Transmitting file data ...
Committed revision 1557418.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #11 from Kevin A. McGrail <km...@pccc.com> ---
I also added much of the text from the announcement to the redis.pm so it's
available as documentation.
Can you add some documentation to the top of the Util::TinyRedis.pm when you
get a moment explaining about how it might be useful, etc.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #9 from Mark Martinec <Ma...@ijs.si> ---
> Can't locate object method "new"
> via package "Mail::SpamAssassin::BayesStore::TinyRedis"
added a missing "use Mail::SpamAssassin::BayesStore::TinyRedis"
Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
Committed revision 1557372.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #4 from Mark Martinec <Ma...@ijs.si> ---
> svn commit -m 'renamed Redis Package to match filename to pass xt test - Bug
> 6990' lib/Mail/SpamAssassin/BayesStore/Redis.pm
> Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
> Transmitting file data .
> Committed revision 1557256.
Hmmm. Does this really work? There are two different
methods 'new' in this file, and now the later overrides
the first ... but we need both! This is why they were
in separate namespaces.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.4.1 |3.4.0
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
--- Comment #7 from Mark Martinec <Ma...@ijs.si> ---
> Adding lib/Mail/SpamAssassin/BayesStore/TinyRedis.pm
A better place for this would be
lib/Mail/SpamAssassin/Util/TinyRedis.pm
so that it can be re-used for other purposes in the future.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
CC| |kmcgrail@pccc.com
Resolution|--- |FIXED
--- Comment #3 from Kevin A. McGrail <km...@pccc.com> ---
After considering this more, renaming a module would really require a 3.5
release not a 3.4.1 so now really is the time to get this done.
If it causes an issue, we can revert but I'd rather pass all the tests we have
documented for a release and get this thing out the door.
svn commit -m 'renamed Redis Package to match filename to pass xt test - Bug
6990' lib/Mail/SpamAssassin/BayesStore/Redis.pm
Sending lib/Mail/SpamAssassin/BayesStore/Redis.pm
Transmitting file data .
Committed revision 1557256.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
Mark Martinec <Ma...@ijs.si> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.4.0 |3.4.1
Severity|normal |enhancement
--- Comment #1 from Mark Martinec <Ma...@ijs.si> ---
Yes, I'm aware of it, Kevin pointed it out on 2013-10-11.
I prefer not to touch this before the 3.4.0 release. It's just a
'good programming practice issue', otherwise it's nothing wrong
with changing namespaces within a single perl module.
Eventually the Redis module could make it into a separate file
under Util/ when there is a need to use it for some other task
besides the bayes database. Now that we are in the 3.4.0-RC phase
this is not the best time for such reorganization - it will have
to wait till next release IMO.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 6990] Redis.pm fails perlcritic test for package name
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6990
Joe Quinn <jq...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jquinn+SAbug@pccc.com
Severity|enhancement |normal
--- Comment #2 from Joe Quinn <jq...@pccc.com> ---
Sounds good. This was just on Kevin's notes for RC4. I'll update our notes to
reflect the ticket.
--
You are receiving this mail because:
You are the assignee for the bug.