Review Request 127655: Fix KAboutData::applicationData() to init from current Q*Application metadata

[Friedrich W. H. Kossebau]

> On May 4, 2016, 3:40 a.m., Michael Pyne wrote:
> > autotests/kaboutdatatest.cpp, line 317
> > <https://git.reviewboard.kde.org/r/127655/diff/2/?file=462407#file462407line317>
> >
> >     Doesn't this call make the KAboutData::applicationData() call that happens later return *this* "aboutData" object (contrary to its comment)?
> >     
> >     Might be better to combine these two test cases into one that sets the Qt data, verifies it's picked up into a KAboutData, and then with a new KAboutData object, calls setApplicationData and verifies that the QCoreApplication data settings are updated.

Turned those two tests into one now, and moved into separate test case, to improve protection against side-effects of other (future) tests cases in kaboutdatatest.
Also included additional checking of roundtrip with setApplicationData() and applicationData().


> On May 4, 2016, 3:40 a.m., Michael Pyne wrote:
> > src/lib/kaboutdata.cpp, line 925
> > <https://git.reviewboard.kde.org/r/127655/diff/2/?file=462409#file462409line925>
> >
> >     Please either make this a static function or place it in an anonymous namespace, no need to make this symbol visible in the compiled object file or library.

Put it into anonymous namespace, for consistency with other helper methods in kcoreaddons.


- Friedrich W. H.


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


On May 4, 2016, 2:47 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127655/
> -----------------------------------------------------------
> 
> (Updated May 4, 2016, 2:47 p.m.)
> 
> 
> Review request for KDE Frameworks, Alex Richardson and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), without requiring the user to use KAboutData::setApplicationData(). So better be complete when initializing the data from the Q*Application metadata.
> 
> Also
> - warn if there is no Q*App instance yet to set properties in KAboutData::setApplicationData
> - check and warn if KAboutData::applicationData is out-of-sync with qapp data
> - remove bogus check for empty display name, as the method defaults to componentname
> - unit tests KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt a7a6752 
>   autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION 
>   src/lib/kaboutdata.h 97c0f2b 
>   src/lib/kaboutdata.cpp ceb0c06 
> 
> Diff: https://git.reviewboard.kde.org/r/127655/diff/
> 
> 
> Testing
> -------
> 
> Added autotests pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

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