Review Request 117275: Deprecate the catalog name stuff from KAboutData

[David Faure]

> On April 12, 2014, 9:16 a.m., David Faure wrote:
> > Ship It!
> 
> David Faure wrote:
>     Well, I guess the first diff minimizes the porting effort indeed.
>     
>     Also: the domain name can only be passed to QCoreApp if this is the main aboutdata (we also have a KAboutData per plugin).
>     But yeah, that seems to be missing in KAboutData::setApplicationData.
> 
> Alex Merry wrote:
>     I could also combine them and add both the short-form and long-form constructors.  I guess the question is: do we want to push users towards a particular style?  The "short constructors and setters" is what Qt has been moving towards, on the basis it makes more readable code; do we want to do the same with this class?
>     
>     The organization domain stuff has a fallback to "kde.org"; currently this doesn't really matter, as we don't actually do anything with it.  But if we're passing it to QCoreApplication, we should think about in the frameworks world (with a hopefully wider audience for these frameworks) we really want that default there.
>     
>     It's easy enough to make the homepage setter change the organization domain if the org dom was never explicitly set (a bool flag would do the trick).

Right, let's have both.

Domain name: if unset by app, whether it's kde.org or empty string will be wrong for apps that should get something else, so.... I would leave the default as is, it caters to the current majority of users of this code, and others have to set something anyway.

Getting it from the homepage setter is probably fine though.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117275/#review55486
-----------------------------------------------------------


On April 1, 2014, 10:09 a.m., Alex Merry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117275/
> -----------------------------------------------------------
> 
> (Updated April 1, 2014, 10:09 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, Kevin Ottens, and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> This is another thing on the "if only we'd spotted it before beta1" list.  I went with not allowing any optional arguments to the constructor, to encourage users of the class to use setters, which makes for more readable code.  I deliberated giving it just one argument, but in the end went with the formerly-required arguments.
> 
> The organizationDomain is not automatically set from the home page with this new usage style, as that only happened in the constructor and not in the setter.  It could be set if the organizationDomain has not been explicitly set.  However, the organizationDomain is not passed to QCoreApplication as I assumed it would be - it that intentional?
> 
> 
> Deprecate the catalog name stuff from KAboutData
> 
> This is pretty useless - the translation catalog has to be set before
> KAboutData is constructed in order to translate its arguments.
> 
> 
> Diffs
> -----
> 
>   src/lib/kaboutdata.h cff1e3f67e33657fdd265a82166ef2a04cbcc3d1 
>   src/lib/kaboutdata.cpp ce64a13aaa89bb4bc077f05e5f8e175d6a441ead 
> 
> Diff: https://git.reviewboard.kde.org/r/117275/diff/
> 
> 
> Testing
> -------
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140412/9890bd01/attachment.html>