[Differential] [Requested Changes To] D3597: Investigate warnings and fix them [3]

[kfunk (Kevin Funk)]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


  Rest LGTM

INLINE COMMENTS

> codecompletionitemgrouper.cpp:28
>  ///@todo make configurable. These are the attributes that can be respected for grouping.
> -const int SimplifiedAttributesExtractor::groupingProperties = CodeCompletionModel::Public | CodeCompletionModel::Protected | CodeCompletionModel::Private | CodeCompletionModel::Static | CodeCompletionModel::TypeAlias | CodeCompletionModel::Variable | CodeCompletionModel::Class | CodeCompletionModel::GlobalScope | CodeCompletionModel::LocalScope | CodeCompletionModel::GlobalScope | CodeCompletionModel::NamespaceScope;
> +const int SimplifiedAttributesExtractor::groupingProperties = CodeCompletionModel::Public | CodeCompletionModel::Protected | CodeCompletionModel::Private | CodeCompletionModel::Static | CodeCompletionModel::TypeAlias | CodeCompletionModel::Variable | CodeCompletionModel::Class | CodeCompletionModel::GlobalScope | CodeCompletionModel::LocalScope | CodeCompletionModel::NamespaceScope;
>  

Style: While at it: Let's do one enum value per line?

> ematirov wrote in mainwindow_p.cpp:321
> 1. There is no need in moving this line
> 2. "action = " part can be omitted since it's not used anyway.
> 
> But maybe it's better to keep it as it is since it doesn't hurt and do not create not very needed change in history. @kfunk?

Please a) don't move the line, b) just omit `action =`. That's fine.

> ematirov wrote in test_path.cpp:373
> Probably, this warning was false-positive warning since this one tests if equality overload is really working at all

+1, don't remove these lines

@ematirov Is there a way to instruct PVS to ignore certain lines?

> ematirov wrote in vcspluginhelper.cpp:427
> Not really necessary probably. @kfunk, same question?

Change looks good to me. I'd keep it.

REPOSITORY
  R33 KDevPlatform

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

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

To: Sergobot, ematirov, kfunk
Cc: ematirov, kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161205/faa97858/attachment-0001.html>