[Differential] [Updated] D862: [Fixed Bug 183625] Sorting of files in the commit dialog

[kfunk (Kevin Funk)]

kfunk added a comment.


  Just lot of nitpicks. Good work!

INLINE COMMENTS
  plugins/patchreview/patchreviewtoolview.cpp:209 This looks quite expensive.
  
  Try setting http://doc.qt.io/qt-5/qsortfilterproxymodel.html#dynamicSortFilter-prop to true in the proxy model
  plugins/patchreview/patchreviewtoolview.h:101 Use more descriptive name, e.g. `m_fileChangesProxyModel`
  vcs/models/vcsfilechangesmodel.cpp:104 `return QVariant::fromValue(m_info.state())`.
  
  You will have to `Q_DECLARE_METATYPE(VcsStatusInfo::State)` in the header
  vcs/models/vcsfilechangesmodel.h:54 Name `enum Column { ... }`
  vcs/models/vcsfilechangessortproxymodel.cpp:29 Please keep the argument names as in the base class: `source_left` and `source_right`. There here to disambiguate between source & proxy model indices.
  vcs/models/vcsfilechangessortproxymodel.cpp:36 Simplify: `source_left.data(VcsFileChangesModel::StateRole).toInt()` should work
  vcs/models/vcsfilechangessortproxymodel.cpp:46 Dito, see above.
  vcs/models/vcsfilechangessortproxymodel.cpp:49 Style: `return a < b`
  vcs/models/vcsfilechangessortproxymodel.cpp:53 Please add newline at EOF
  vcs/models/vcsfilechangessortproxymodel.h:34 Use `QObject* parent = nullptr` (same as baseclass)
  vcs/models/vcsfilechangessortproxymodel.h:35 Add `override`

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

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

To: apuzio, kfunk
Cc: kdevelop-devel