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

[coliveira (Carlos Nihelton)]

coliveira marked 6 inline comments as done.
coliveira added a comment.


  Thanks for the comments, guys. I already made most of the suggestions, I think.
  About the KConfigSkeleton  approach I need to study more to understand how it works. In the meantime, I left the PerProjectConfigPage almost in the same way it was, except for the pointer API, which Kevin Funk suggested to change.
  As soon as I understand the KConfigSkeleton enough to reimplement the PerProjectConfigPage I'll submit another patch.
  
  Thank you, guys!

INLINE COMMENTS

> kfunk wrote in ClangFormatAll.cmake:13
> Does that need to be an error?

Well, WARNING should be enough, since it's just for developers to auto format code.

> kfunk wrote in perprojectconfigpage.h:65
> I'm not too fond of this API (`QStringList*`). Maybe it makes more sense to use a property for the receptor list and communicate changes via signal/slots?

Now PerProjectConfigPage is the owner of the list of selected checks and it passes this list by value by emitting a signal which is connected to a slot on the Plugin class. Do you think it is enough?

> kfunk wrote in job.cpp:151
> Handling those QProcess errors should probably be moved to the KDevelop::OutputExecuteJob class in kdevplatform?

I saw KDevelop::OutputExecuteJob  code and realized that I was doing nothing special, so I preferred to drop this method.

> antonanikin wrote in plugin.cpp:251
> it's better to use new API:
> `core()->languageController()->problemModelSet()->showModel(problemModelName);`

Cleaner API. I like it.

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/20161003/3eb21873/attachment.html>