Mailing List Archive

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From felicity@kluge.net 2004-05-27 21:30 -------
*** Bug 3437 has been marked as a duplicate of this bug. ***



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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438

quinlan@pathname.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.1.0 |3.0.0





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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From felicity@kluge.net 2004-06-02 17:04 -------
ok, so now I'm kind of confused too.

M::SA::learn() is what people should be calling.

a PML object is returned so that people can do more if they want to, but I'm not really sure what the
point is to that. does anyone actively use PML? the only thing I found was PMS and sa-learn calls
did_learn(), which could easily be turned into a return value from M::SA::learn -- then we could get rid
of PML.


alternately, the idea is that like PMS, PML lets people generate the object using a static API, and then
they can do stuff with it if they want to or just call finish. as such, they should call M::SA::learn() with
only the message as a parameter (per the docs) and then they can call learn_spam, learn_ham, etc.


in short, it depends what you're trying to do, but M::SA::learn() is the way to go always.


thoughts?


Since all the PML stuff is doable via M::SA::learn() and the options that are passed, I'd rather just get rid
of PML and simplify the whole thing, but I'm thinking I'm missing something which makes keeping it
around a good idea.



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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From jm@jmason.org 2004-06-02 17:13 -------
Subject: Re: Conflicting API documentation

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


>a PML object is returned so that people can do more if they want to, but
>I'm not really sure what the point is to that. does anyone actively use
>PML? the only thing I found was PMS and sa-learn calls did_learn(),
>which could easily be turned into a return value from M::SA::learn --
>then we could get rid of PML.

It's good OO practice to have an object returned in this case, similar
to how the PerMsgStatus object works. That's what PerMsgLearner
is there for; a public "state of that learn request" return type.

We could alternatively return a "did_learn" bool from
M::SpamAssassin::learn() -- but what happens then if we need to provide
*more* info to callers, in the same way the PMS has additional methods
beyond get_hits(), such as get_report(), etc.? hence, better to have an
object, so we can increase the data that gets returned without having to
break old APIs.

>alternately, the idea is that like PMS, PML lets people generate the
>object using a static API, and then they can do stuff with it if they
>want to or just call finish. as such, they should call M::SA::learn()
>with only the message as a parameter (per the docs) and then they can
>call learn_spam, learn_ham, etc.

this isn't a good plan. Better for PerMsgLearner to be the returned
"state of your learn request" object, and not reusable.

The real problem is purely that the (internal) APIs on PerMsgLearner
are being documented in POD sections. they shouldn't be -- or at
least it should be made clear that those aren't public APIs.

I have a patch for this -- uploading in a sec...

- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Exmh CVS

iD8DBQFAvmztQTcbUG5Y7woRAh4jAJ0SctAv+yKYPy52zhwrbIQTUm1IQACgt946
rAXJdq8eudkVHdIhJFz9m8I=
=FeU+
-----END PGP SIGNATURE-----





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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From jm@jmason.org 2004-06-02 17:15 -------
Created an attachment (id=1992)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=1992&action=view)
proposed patch




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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From duncf@debian.org 2004-06-02 21:45 -------
-0.1

I think it'd be better to simply leave the internal API out of the
documentation. Simply turn the POD sections into internal comments.
Documentation shouldn't explain how stuff is done internally, it should only
describe the interface that is meant to be used.



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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438

jm@jmason.org changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #1992 is|0 |1
obsolete| |



------- Additional Comments From jm@jmason.org 2004-06-02 22:34 -------
Created an attachment (id=1996)
--> (http://bugzilla.spamassassin.org/attachment.cgi?id=1996&action=view)
revised

ok, makes sense. try this one...



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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From quinlan@pathname.com 2004-06-03 20:22 -------
+1 looks good to me

side comment: I'd be a happier camper if the external API did not have
the isspam flag and instead had a class flag which would initially accept
"ham" or "spam". on/off is the wrong semantics for the long-run.





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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From duncf@debian.org 2004-06-03 20:27 -------
Subject: Re: Conflicting API documentation

+1 for me too. (although doc changes probably dont need review)

> side comment: I'd be a happier camper if the external API did not have
> the isspam flag and instead had a class flag which would initially accept
> "ham" or "spam". on/off is the wrong semantics for the long-run.

It's probably too late to change now, isn't it?




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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From jm@jmason.org 2004-06-03 21:11 -------
Subject: Re: Conflicting API documentation

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


>> side comment: I'd be a happier camper if the external API did not have
>> the isspam flag and instead had a class flag which would initially accept
>> "ham" or "spam". on/off is the wrong semantics for the long-run.
>
>It's probably too late to change now, isn't it?

it is. this API has been that way since 2.50. however, Dan's right
for the long run, so new APIs should do something like that...

- --j.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Exmh CVS

iD8DBQFAv/ZHQTcbUG5Y7woRArqrAKDMzsHlAAuZH7sJmHUWjRG8hO+VUQCfd+vZ
d1L8k9LL83GyCRBsvTNQxpE=
=Izrv
-----END PGP SIGNATURE-----





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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From felicity@kluge.net 2004-06-04 13:55 -------
+1

ok, if the PML methods are private, then I'm fine with the patch. they looked to be public which is what
confused the whole matter.



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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From duncf@debian.org 2004-06-04 15:07 -------
Subject: Re: Conflicting API documentation

> ok, if the PML methods are private, then I'm fine with the patch. they looked to be public which is what
> confused the whole matter.

Of course, in perl, the difference between public and private methods are whether or not they're documented ;-)




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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438





------- Additional Comments From felicity@kluge.net 2004-06-04 17:09 -------
Subject: Re: Conflicting API documentation

On Fri, Jun 04, 2004 at 03:07:35PM -0700, bugzilla-daemon@bugzilla.spamassassin.org wrote:
> Of course, in perl, the difference between public and private methods are whether or not they're documented ;-)

by convention, private functions start with an underscore (_).





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

http://bugzilla.spamassassin.org/show_bug.cgi?id=3438

jm@jmason.org changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED



------- Additional Comments From jm@jmason.org 2004-06-04 18:16 -------
ok -- applied



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