Review Request 111910: Deprecate unused methods + enum values in KEMailSettings

Martin Klapetek martin.klapetek at gmail.com
Thu Aug 8 08:01:01 UTC 2013



> On Aug. 7, 2013, 4:25 p.m., David Faure wrote:
> > Thanks for that review.
> > 
> > So, profiles() and setProfile() are used? For which purpose?
> 
> Martin Klapetek wrote:
>     profiles() is used quite extensively in kdepim, it iterates over profiles looking for an email address (aka "is this particular email address among user's email addresses"), see http://lxr.kde.org/source/kde/kdepimlibs/kcal/incidenceformatter.cpp#117 for example.
>     
>     setProfile() is imho a bit misued, it's used as 
>     
>     KEMailSettings es;
>     es.setProfile( es.defaultProfileName() );
>     
>     however this already happens in constructor. These are the only usages of setProfile() I've found, so I'd be happy to deprecate it as well as it's a bit redundant. I didn't because it breaks quite a lot of code, on the other hand I think that's expected with kf5...
> 
> David Faure wrote:
>     Ah, I see. Nasty stateful API, having to "setProfile" in order to read the settings (e.g. email) in that profile. Could be improved, but OK, not worth it.
>     
>     I'm still wondering if any code actually *writes* into multiple "profiles".
>     kdepimlibs' IdentityManager assumes KEMailSettings only has the "default profile", i.e. it only writes out the default identity into kemailsettings.
>     But OK, that could be considered a bug, since other code (e.g. the one in kcal) assumes that all emails will be in profiles.
>     
>     OK, so bad API and incomplete usage, but we can't get rid of it :-)

> I'm still wondering if any code actually *writes* into multiple "profiles".

I wondered the same and I haven't found any writes to other profiles, everyone is just setting the default profile (I wonder if it was added to the constructor only after many people were unhappy having to set what should be the default or they simply didn't know...either way, I'll update the documentation to say that this is happening).


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111910/#review37292
-----------------------------------------------------------


On Aug. 6, 2013, 2:30 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111910/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2013, 2:30 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> I've looked through all usages of KEMailSettings on lxr, deprecated things in diff are nowhere used.
> 
> 
> Diffs
> -----
> 
>   staging/kio/src/core/kemailsettings.h 873c222 
>   staging/kio/src/core/kemailsettings.cpp 39dc9f7 
> 
> Diff: http://git.reviewboard.kde.org/r/111910/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130808/1fcba314/attachment.html>


More information about the Kde-frameworks-devel mailing list