More on API changes

[Justin Mason <jm...@jmason.org>]

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


Malte S. Stretz writes:
> On Thursday 05 February 2004 23:24 CET Daniel Quinlan wrote:
> > Justin Mason <jm...@jmason.org> writes:
> > > We've already changed the API (NoMailAudit -> MessageParser). ;)
> >
> > Okay, I'll change my vote to a +1 if we can have a discussion about
> > renaming some other modules and making sure we're sure about the names.
> 
> Read this list ;-)
> 
> Especially that message called "Plans for 3.0?", written by me on 
> 21.01.2004. Justin and Duncan already replied relatively positively. I'd 
> went start to implement the stuff I suggested there in a few days in a 
> branch anyway so you'd have noticed :-)

BTW I've been thinking about the proposed renaming of some of the major
"UI" modules -- in particular NoMailAudit -> MessageParser, and it's API
change in its constructor.

IMO, we *need* to keep compatibility on this.   It's the main API used by
users of the perl modules, and I've just had to fix a third-party script
that used it; there's no easy way to support multiple versions of the
SpamAssassin modules without doing:

	my $msg;
	if ($Mail::SpamAssassin::VERSION > 2.69) {
	  $msg = new Mail::SpamAssassin::MessageParser (\@lines);
	} else {
	  $msg = new Mail::SpamAssassin::NoMailAudit (data => \@lines);
	}

This is a very nasty thing to do for a public API. :(

So I recommend we either

1. keep NoMailAudit for the name of that module, and its API (preferred!)

2. add a NoMailAudit compatibility wrapper class (using the "facade"
pattern) which is used to wrap Mail::SpamAssassin::MessageParser.

Basically, this is the *main* public API for Mail::SpamAssassin.
Breaking it will cause a lot of pain for users.


Also -- another issue: the naming, MessageParser, is not appropriate
IMO.  It seems good from our point of view from "inside" the product,
but for users of the Mail::SpamAssassin modules, it makes little
sense; they use that class as a message object, not as a message-parser
object.  (it just performs message-parsing operations "behind the
scenes" for us.)

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

iD8DBQFAI9pGQTcbUG5Y7woRAmHAAJ926tXDfdi20kS4w9j71TvTDONyCQCeNL29
CglJ5ILexiE83Odc+Rfs754=
=PCqZ
-----END PGP SIGNATURE-----

Re: More on API changes

[Michael Parker <pa...@pobox.com>]

On Fri, Feb 06, 2004 at 03:34:30PM -0500, Theo Van Dinter wrote:
> 
> I don't have a problem with the API change as long as we continue
> "cleaning house" and getting rid of the old baggage that we don't
> need/want to keep around.
> 

Just to put in my $.02.  I'm all for an API change, as long as it
accompanies an improvement in the underlying code.  The drawbacks to
changing the API are that third-party applications must be updated to
work correctly and sometimes this can burn a project (see Apache 2.X
vs Apache 1.3).  I think you can overcome this drawback by providing a
significant incentive for third-party tools to update their API.  SA
sorta has this builtin because without updating you don't get the
newer more effective rules.  I don't think it would fall into the
Apache category where everything works well on 1.3 so why update to
the newer, more complicated API.

If now is a good opportunity to clean up the existing API so that it's
a) easier to work on underlying code and b) makes more sense, then I
suggest we take it.  Might as well go in head first, and do as much
API changing now rather than having to go through it all again a few
months down the road.

I can see the use for backward compatability, but I'd make it strictly
a wrapper which perhaps issues a warning each time it's called, "Please
upgrade to the new API, thank you," and elimiate it after one or two
point releases.

Michael

Re: More on API changes

[Theo Van Dinter <fe...@kluge.net>]

On Fri, Feb 06, 2004 at 10:17:42AM -0800, Justin Mason wrote:
> IMO, we *need* to keep compatibility on this.   It's the main API used by
> users of the perl modules, and I've just had to fix a third-party script
> that used it; there's no easy way to support multiple versions of the

Well, that depends ...  How important is backward compatibility to us?
I don't want to change everything around for the fun of it, but I want
to get rid of as much cruft as I can.  For instance, getting rid of the
whole Mail::Audit stuff made "NoMailAudit" unnecessary.

> This is a very nasty thing to do for a public API. :(

Yes and no.  It's a nasty thing to do unless a major release occurs
where APIs are allowed to change.  Hence the whole 3.0 vs 2.7 thing.

> 1. keep NoMailAudit for the name of that module, and its API (preferred!)

We could keep the name, but the API will be different unless we
do a bit of reengineering.  In fact, I don't really think this is
possible. (MsgContainer->NoMailAudit)

> 2. add a NoMailAudit compatibility wrapper class (using the "facade"
> pattern) which is used to wrap Mail::SpamAssassin::MessageParser.

Could do that, but it'll be really annoying, especially since NoMailAudit
will just be around for backwards compatibility with 3rd party modules,
while our code uses the new API.  Not to mention the fact that some stuff
from NoMailAudit just isn't valid anymore (mostly an issue for people
who like to access object variables directly and not through the API.)

There are 2 main issues with the current design that make the old API
break.  1) You call MsgParser and get back a MsgContainer object.  There's
really no point in having a MsgParser object since it's just making a tree
of MsgContainer nodes.  2) M::SA::PerMsgStatus::rewrite_mail() returns a
_NEW_ MsgContainer tree.  The reasons for this are 1) it's cleaner, and 2)
there's really no easy way of recreating the tree from the tree itself.

> Also -- another issue: the naming, MessageParser, is not appropriate
> IMO.  It seems good from our point of view from "inside" the product,

Yeah, whatever.  I picked MsgParser and MsgContainer for lack of
better names at the time.  If we wanted to be accurate, MIME::Parser
and MIME::TreeNode would probably be better.  I don't really like those
either though.  Feel free to come up with new names for those, it's not
hard to change.


I don't have a problem with the API change as long as we continue
"cleaning house" and getting rid of the old baggage that we don't
need/want to keep around.

-- 
Randomly Generated Tagline:
"Running Linux 1.2 Because a 486 is a terrible thing to waste." - Unknown