> 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.
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). - Alex ----------------------------------------------------------- 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
