[Differential] [Updated] D2840: First diff for kdev-clang-tidy

[antonanikin (Anton Anikin)]

antonanikin added a comment.


  Looks good, thanks.
  
  I also had a very brief first look, and my remarks are:
  
  1. Your TODO about removing hard-coded CLANG_TIDY_PATH - good idea, do it.
  
  2. Check code reworked from kdev-cppcheck for unused parts. My main remark - confusing comments, which seems to be an incorrect for your app (see job.cpp for example).
  
  3. Per-project configuration code looks a bit complicated and redundant. I think it will be good to reimplement per-project configuration with using standard KConfigSceleton approach.  This way allows you to drop(or simplify) some "boring" code like a manually created `apply()` methods for Config Pages. My last version of kdev-cppcheck (differential D2844 <https://phabricator.kde.org/D2844>) already uses this approach.

INLINE COMMENTS

> plugin.cpp:251
> +
> +        core()->uiController()->findToolView(i18nd("kdevproblemreporter", "Problems"), nullptr,
> +                                             KDevelop::IUiController::FindFlags::Raise);

it's better to use new API:
`core()->languageController()->problemModelSet()->showModel(problemModelName);`

REPOSITORY
  R218 KDev Clang-Tidy Support

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

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

To: coliveira, mwolff, apol, kfunk, antonanikin
Cc: kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161001/ca78e5a3/attachment.html>