[Kde-pim] Review Request: Fix for bug 190232

[Thomas McGuire]

Hi,

On Tuesday 16 June 2009 22:56:58 Ingo Klöcker wrote:
> On Tuesday 16 June 2009, Thomas McGuire wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/848/#review1325
> > -----------------------------------------------------------
> >
> > Ship it!
> >
> >
> > Thanks for catching this, if seen quite some bug reports about it!
> > I committed it in r982805, but a bit different:
> >
> > if ( mUserWhoField == whoField && !whoField.isEmpty() )
> >   return;
> >
> > Side note: I think using a QString for the "who field" here instead
> > of an enum is horrible style, but this can't be changed because of
> > config compatibility, except if one writes an upgrade script.
>
> Enums should always be stored in string representation in config because
> this prevents stupid (and potentially severe) errors due to changes
> made to the enums. In KMail's past such a stupid error lead to message
> loss due to a changed enum value in the POP filter code: Messages were
> deleted from the POP server instead of being deferred because the
> meaning of the int value stored in the config file had changed.
>
> Ideally, the string representation of the enums should only be used when
> reading/writing the configuration. All other code/API should use the
> integer representation of the enums.
>
> So you do not have to write an upgrade script. All you have to do is to
> convert the string representation of the "who field" to an integer
> representation when reading/writing the configuration. The good thing
> is that with the help of QMetaEnum the code doing the conversion can be
> written in a couple of lines (including proper handling of unknown enum
> values).

I think in this case you still need an upgrade script, since the empty string 
also has a meaning. Otherwise, I agree.

Regards,
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20090616/1ab0db66/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/