D17794: Pkg-config plugin

[Gleb Popov]

arrowd added inline comments.

INLINE COMMENTS

> definesandincludesprovider.cpp:89
> +    Defines *outDefines,
> +    Path::List *outIncludes,
> +    QStringList *outPackages )

Why not pass by reference? This would eliminate need to `*outIncludes` in the body.

> definesandincludesprovider.cpp:90
> +    Path::List *outIncludes,
> +    QStringList *outPackages )
> +{

Same thing.

> kdevpkgconfigmanager.json:17
> +    "X-KDevelop-LoadMode": "AlwaysOn",
> +    "X-KDevelop-Mode": "NoGUI"
> +}

Since this plugin has graphical windows, should this really be `NoGUI`?

> pkgconfigmanager.cpp:55
> +    KDevelop::IDefinesAndIncludesManager::manager()->registerProvider(
> +        SettingsManager::globalInstance().provider() );
> +}

It is a bit strange that you store `DefinesAndIncludesProvider*` in `SettingsManager`. Why not add `static DefinesAndIncludesProvider::getInstance()`, like classic Singleton pattern?

REPOSITORY
  R32 KDevelop

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

To: blackwarthog, #kdevelop
Cc: apol, rjvbb, arrowd, kdevelop-devel, #kdevelop, glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190103/94ef2cf2/attachment-0001.html>