[Differential] [Commented On] D2925: Port kdev_format_source script to C++/Qt

[kfunk (Kevin Funk)]

kfunk added a comment.


  I'll give it a runtime ASAP, then I can give the final 'go'

INLINE COMMENTS

> kdevformatfile.cpp:155
> +
> +#if defined(_WIN32) or defined(_WIN64)
> +    int execResult = QProcess::execute("cmd", {"/c", command});

`#ifdef Q_OS_WIN`? (Qt way)

> kfunk wrote in test_kdevformatsource.cpp:32
> Hmm, test looks useful, but it would be more easy to understand if you'd have used data-driven tests here. In the data section, you could both specify the format file contents + expected contents for the source files.
> 
> Right know we're duplicating a lot of code in the test functions.

I actually meant the way Qt implements data-driven testing, see here: http://doc.qt.io/qt-5/qttestlib-tutorial2-example.html

But feel free to leave it like that, I'll refactor it if noone else steps up.

> test_kdevformatsource.cpp:32
> +
> +void TestKdevForamtSource::doTest(const QStringList& formatFileData, bool hasCommands, bool hasMatch)
> +{

Typo: TestKdevForamtSource

REPOSITORY
  rKDEVPLATFORM KDevPlatform

REVISION DETAIL
  https://phabricator.kde.org/D2925

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: antonanikin, #kdevelop, kfunk
Cc: kfunk, apol, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161018/6fd20292/attachment.html>