Created attachment 2842 [details]
fix

ok, here's a patch that does this -- it passes 'make test', but I'd like some
more eyes before I check it in.

key points:
  - it keeps the Mail::SpamAssassin::copy_config() public API compatible with
3.0.0's, or at least with what the POD documentation says. ;)  this is
important, because after all it was a public API in that release.
  - however, the implementation has been moved to Mail::SpamAssassin::Conf,
since it was an Official OO Bad Thing for Mail::SpamAssassin code to have to
know so much about the internal structure of the Conf class of objects.  it's
cleaner ot have that in Conf.
  - there's no use of Storable left anywhere in the main distro (outside of
masses/rule-qa). yay!

Subject: Re:  remove use of Storable in spamd

It looks good for the most part.  However, I don't particularly like the
special cases.  I'd rather all the data in conf was handled
consistently.  I'm okay with objects that need to be cloned, but I don't
like special-casing them in the copy code.  It should be clear what do
to by how they are defined in conf.

'It looks good for the most part.  However, I don't particularly like the
special cases.  I'd rather all the data in conf was handled
consistently.  I'm okay with objects that need to be cloned, but I don't
like special-casing them in the copy code.  It should be clear what do
to by how they are defined in conf.'

not sure how you'd intend to do that ;)   could you explain?

Subject: Re:  remove use of Storable in spamd

> not sure how you'd intend to do that ;)

A naming convention might work.

  ->{conf}->{object}

and anything under that would have the call.  There's probably a better way.

> could you explain?

Well, I really do not like these special cases:

     elsif ($k =~ /^(internal|trusted)_networks$/) {

     if ($v->{nets}) {

     elsif ($k eq 'scores') {

Subject: Re:  remove use of Storable in spamd 

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


> A naming convention might work.

good point -- I hadn't thought of that.  OK, let me have a try...

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

iD8DBQFCeplsMJF5cimLx9ARAt69AKCPH/MOf2KrK1kRGVMi1r7te+JIpwCfdif8
JVlIhspSjVbht6zBuEkfviw=
=F5Zo
-----END PGP SIGNATURE-----

Created attachment 2848 [details]
revised fix

ok, here's a revised version; it fixes a couple of bugs, simplifies the clone()
code a little, and hopefully makes the code a bit less special-case-y.

Daniel, I looked into the idea of making the name reflect the scoping, but that
increases overhead quite a lot -- it means that all the code that *uses* that
variable has to change in (possibly multiple) places to deal with cleaning up a
special case in one, single, well-defined place.  doesn't make sense...

checked in -- r169246.