[Differential] [Commented On] D3580: Investigate warnings and fix where it's possible and needed (kdevplatform) [1]

ematirov (Mikhail Ivchenko) noreply at phabricator.kde.org
Sat Dec 3 17:12:31 UTC 2016


ematirov added inline comments.

INLINE COMMENTS

> ematirov wrote in expandingdelegate.cpp:134
> Please use "NotExpandable" there (and in other cases) instead of constant there since it's enum. The main idea of problem there is that order of values in enum can be changed, so NotExpandable will become no-zero and all these ifs will not work as expected.

And that's one, please. It's really better to use enum by name and not by value.

> ematirov wrote in testview.cpp:243
> Let's use just !stditemlist.IsEmpty() and stditemlist.first().
> And probably "itemsForProject" or something like so will be more meaningful name for that. ;)

It should be !itemsForProject.isEmpty(), not just itemsForProject.isEmpty(). Now it works like:
Check if list is empty (does not contain items) and if it's empty return first item; which is incorrect.

REPOSITORY
  R33 KDevPlatform

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

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

To: spencerb, #kdevelop, kfunk, ematirov
Cc: kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161203/4d7d57fb/attachment.html>


More information about the KDevelop-devel mailing list