Review Request 112984: Adjust API in KEmoticons framework: KEmoticonsProvider::save()

[David Gil Oliva]

> On Sept. 30, 2013, 10:48 a.m., Aleix Pol Gonzalez wrote:
> > staging/kemoticons/src/core/kemoticonsprovider.h, line 95
> > <http://git.reviewboard.kde.org/r/112984/diff/1/?file=193005#file193005line95>
> >
> >     Use KEMOTICONS_DEPRECATED

What a stupid mistake!!! Sorry :-/


On Sept. 30, 2013, 10:48 a.m., David Gil Oliva wrote:
> > I don't really understand what's better from ::save instead of ::saveTheme. Is it just the naming? They seem to do the same.

Yes, it is "just" the naming, which I consider to be very important for the users of the API. I think it's more consistent to have saveTheme() because there's already loadTheme() and newTheme(). With save() one doesn't know what is being saved :-D

Likewise, in this framework one can find newTheme() and createNew(), which I would unify into only newTheme():

I'm trying to make the whole API more consistent and clearer.


- David


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


On Sept. 28, 2013, 9:19 p.m., David Gil Oliva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112984/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2013, 9:19 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> Adjust API in KEmoticons framework: KEmoticonsProvider::save()
> 
> -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::save()
> and prefer saveTheme() instead.
> -Adjust plugins.
> -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::save(),
> which was empty. Now it's a pure virtual function. Therefore, I deprecate
> it and advice subclassing KEmoticonsProvider.
> 
> 
> Diffs
> -----
> 
>   staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 
>   staging/kemoticons/src/core/kemoticonstheme.h b8b25f7607d3741dda78d6302c1ed81fbc9211a0 
>   staging/kemoticons/src/core/kemoticonstheme.cpp ed3407cdd45c84d91f2d0057e108b760ff696ff2 
>   staging/kemoticons/src/providers/adium/adium_emoticons.h 039a267679919c968bbe4d12c35abb90cf1bcc9b 
>   staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 
>   staging/kemoticons/src/providers/kde/kde_emoticons.h 8cba6b194eee1543bf13f1177a0e4092f1a10943 
>   staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a 
>   staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h 70bafa3fe4ba25c9d6a69634024b2c157235e674 
>   staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 
>   staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 0873a635a300e0ed8e38b1d59a535d6ec15b99cb 
>   staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 
> 
> Diff: http://git.reviewboard.kde.org/r/112984/diff/
> 
> 
> Testing
> -------
> 
> It builds. It installs. Tests pass.
> 
> 
> Thanks,
> 
> David Gil Oliva
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130930/224aacc4/attachment.html>