D14288: Initial version of Clazy analyzer plugin

Pino Toscano noreply at phabricator.kde.org
Sun Jul 29 13:20:06 BST 2018


pino added inline comments.

INLINE COMMENTS

> CMakeLists.txt:6
> +    DESCRIPTION "Qt oriented code checker based on clang framework. Krazy's little brother"
> +    URL "https://github.com/KDE/clazy"
> +    PURPOSE "Recommended: required by the non-essential Clazy plugin"

github is not the primary location, which is git.kde.org.
Please use: https://commits.kde.org/clazy

> checksdb.cpp:114
> +            check->url = QUrl(QStringLiteral(
> +                "https://github.com/KDE/clazy/blob/master/docs/checks/README-%1.md").arg(check->name));
> +            level->checks[check->name] = check;

Please do not make the plugin phone to github whenever an user wants to read the documentation of a check, it is also a privacy concern.
It looks like clazy installs all the README-$check.md files, so please make use of the locally installed versions. It is also the best option, since you want to read the documentation of the check //you are using//, rather than the latest from the development branch.

> kossebau wrote in commandlinewidget.ui:13
> Unused property injected by Qt designer, please remove (also avoids string to appear for translators).

> Unused property injected by Qt designer, please remove (also avoids string to appear for translators).

Run the `fixuifiles` script that you can find in kde-dev-scripts (also available as kdesdk-scripts in some distros).

> kossebau wrote in globalconfigpage.ui:13
> Unused property injected by Qt designer, please remove (also avoids string to appear for translators).

As said before, `fixuifiles`.

> kossebau wrote in projectconfigpage.ui:13
> Unused property injected by Qt designer, please remove (also avoids string to appear for translators).

As said before, `fixuifiles`.

> kdevclazy.json:6
> +                "Name": "Anton Anikin",
> +                "Name[x-test]": "xxAnton Anikinxx"
> +            }

Any translation, including x-test, will be handled by l10n script -- hence, do not add it manually.

> kdevclazy.json:11
> +        "Description": "This plugin integrates Clazy to KDevelop",
> +        "Description[x-test]": "xxThis plugin integrates Clazy to KDevelopxx",
> +        "Icon": "kdevelop",

Ditto.

> kdevclazy.json:16
> +        "Name": "Clazy Support",
> +        "Name[x-test]": "xxClazy Supportxx",
> +        "ServiceTypes": [

Ditto.

> plugin.cpp:185-187
> +        errorMessage += i18n("Unable to start clazy check for \"%1\":", path);
> +        errorMessage += QStringLiteral("\n\n");
> +        errorMessage += params.error();

This is a string puzzle that can be avoided, since you have the whole message at this point:

  const QString errorMessage = i18n("Unable to start clazy check for \"%1\":\n\n%2", path, params.error());

> problemmodel.cpp:111-114
> +    QString tooltip = i18nc("@info:tooltip", "Re-Run Last Clazy Analysis");
> +    if (m_project) {
> +        tooltip += QString(" (%1)").arg(prettyPathName(m_path));
> +    }

String puzzle -- just use two strings:

  QString tooltip;
  if (m_project) {
    tooltip = i18nc("@info:tooltip %1 is the path of the file", "Re-run last Clazy analysis (%1)", prettyPathName(m_path));
  } else {
    tooltip = i18nc("@info:tooltip", "Re-run last Clazy analysis");
  }

Also tooltips do not get title capitalization.

> test_clazyjob.cpp:82
> +
> +void TestClazyJob::initTestCase()
> +{

Here, make sure to `QSKIP` the whole test if clazy is not installed.

REPOSITORY
  R32 KDevelop

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

To: antonanikin, #kdevelop
Cc: pino, kossebau, kdevelop-devel, antismap, iodelay, vbspam, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180729/1b79c4fe/attachment-0001.html>


More information about the KDevelop-devel mailing list