[Differential] [Commented On] D723: Pimpl DUChain navigation classes

[mwolff (Milian Wolff)]

mwolff added a subscriber: mwolff.
mwolff added a comment.

big +1 from my side in general, but there are some minor issues with this patch as-is.

also note: only commit to master, not to 5.0, as this breaks the ABI.


INLINE COMMENTS
  language/duchain/navigation/abstractdeclarationnavigationcontext.h:37 while at it: please `const&` the `DeclarationPointer` and the `TopDUContextPointer`.
  language/duchain/navigation/abstractdeclarationnavigationcontext.h:38 override
  language/duchain/navigation/abstractnavigationcontext.cpp:44 QVector?
  language/duchain/navigation/abstractnavigationcontext.cpp:56 QHash, also below?
  language/duchain/navigation/abstractnavigationcontext.h:64 `const TopDUContextPointer& topContext = {}`
  language/duchain/navigation/abstractnavigationcontext.h:83 is that really nowhere overwritten, like in kdevphp or ruby or such?
  language/duchain/navigation/abstractnavigationcontext.h:101 const&
  language/duchain/navigation/abstractnavigationcontext.h:103 const&
  language/duchain/navigation/abstractnavigationwidget.h:76 const&

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

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

To: kfunk, KDevelop
Cc: mwolff, kdevelop-devel