If people cant migrate their scripts, 3.0.0 is going to suck ;-)

*** Bug 3437 has been marked as a duplicate of this bug. ***

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.

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-----

Created attachment 1992 [details]
proposed patch

-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.

Created attachment 1996 [details]
revised

ok, makes sense.  try this one...

+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.

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?

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-----

+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.

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 ;-)

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 (_).

ok -- applied