[Differential] [Requested Changes To] D4095: Refactor to embrace a cmake server import backend

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added a comment.
This revision now requires changes to proceed.


  Just some quick review before I go to bed.
  
  Some general remarks:
  
  - Please use categorized logging everywhere
  - Looots of lambdas, wouldn't mind getting rid off at least the ones with bigger "bodies"

INLINE COMMENTS

> cmakeserver.cpp:34
> +{
> +    const QString path = QStandardPaths::writableLocation(QStandardPaths::TempLocation) + "/kdevelop-cmake-" + QString::number(KRandom::random());
> +

`QTemporaryFile`

> cmakeserver.cpp:76
> +
> +void CMakeServer::command(const QJsonObject& object)
> +{

Rename to `sendCommand`?

> cmakeserver.h:32
> +    Q_OBJECT
> +    public:
> +        CMakeServer(QObject* parent);

Style: Deindent here & below

> cmakeserverimportjob.cpp:28
> +
> +template <typename T, typename Q, typename W>
> +static T kTransform(const Q& list, W func)

Style: Off in this whole file, try "Reformat source" with kdev_format script -- you'll be surprised - it works!

> cmakeserverimportjob.cpp:93
> +    const auto configs = response.value("configurations").toArray();
> +    qDebug() << "fuuuuuu" << response;
> +    for (const auto &config: configs) {

How polite :)

> cmakeserverimportjob.h:34
> +{
> +    Q_OBJECT
> +    public:

Style: Deindent here & below

> test_cmakeserver.cpp:3
> + *
> + * Copyright 2006 Matt Rogers <mattr at kde.org>
> + *

He's the author? :)

> testhelpers.h:137
>          qFatal( "Timeout while waiting for opened signal" );
> +        Q_ASSERT(false);
> +    }

Redundant (qFatal aborts already?)

REPOSITORY
  R32 KDevelop

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

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

To: apol, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170112/582c7bdc/attachment-0001.html>