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

[Ingo Klöcker]

On Tuesday 16 June 2009, Thomas McGuire wrote:
> 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.

This could probably be handled by the error handling code (which takes 
care of unknown enum values). The empty string (and any other unknown 
string) would be interpreted as the "default" value.

Of course, an upgrade script would be the better solution.


Regards,
Ingo
-------------- 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/20090617/0da7aa91/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/