Review request

David Faure faure at kde.org
Thu Dec 16 18:58:44 GMT 2010


On Monday 06 December 2010, Thomas Baumgart wrote:
> - a set of unit testcases is provided

However you didn't test compilation of the tests, after moving to kdereview,
which has stricter compilation flags, notably -DQT_NO_CAST_FROM_ASCII.

/d/qt/4/qt-for-trunk/src/corelib/tools/qstring.h: In member function 'void AlkValueTest::stringCtor()':
/d/qt/4/qt-for-trunk/src/corelib/tools/qstring.h:601:5: error: 'QString::QString(const char*)' is private
/d/kde/src/t/kdereview/libalkimia/alkvaluetest.cpp:82:33: error: within this context
/d/qt/4/qt-for-trunk/src/corelib/tools/qchar.h:357:5: error: 'QChar::QChar(char)' is private
/d/kde/src/t/kdereview/libalkimia/alkvaluetest.cpp:82:33: error: within this context
etc.

IMHO it's OK to disable QT_NO_CAST_FROM_ASCII in unittests, you can add
a #undef at the top of the file; or you can use QLatin1String/QLatin1Char in
case this might help detect encoding bugs in the (test) code.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).




More information about the kde-core-devel mailing list