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