This revision was automatically updated to reflect the committed changes.
Closed by commit R120:6d86c690d133: [applets/SystemTray] Implement sorting in
the model (authored by kmaterka).
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D27088?vs=76026&id=
kmaterka updated this revision to Diff 76026.
kmaterka added a comment.
Review fixes
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D27088?vs=75112&id=76026
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D27088
AFFECTED FILES
app
kmaterka added inline comments.
INLINE COMMENTS
> davidedmundson wrote in sortedsystemtraymodel.cpp:47
> I think calling QSortFilterProxyModel::lessThan(left, right); would do the
> same
>
> then you don't need compareDisplayAlphabetically
>
> your implementation looks fine though, so do which
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.
The role names part is nice.
I have one major-ish comment, and 2 pendantic comments that I don't really
care about.
INLINE COMMENTS
> sortedsystemtraymodel.c
kmaterka added a comment.
OK, I will wait for second review from @davidedmundson, @broulik or @mart.
REPOSITORY
R120 Plasma Workspace
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D27088
To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, ngraham, broulik
Cc: mart,
ngraham accepted this revision.
ngraham added a subscriber: mart.
ngraham added a comment.
This revision is now accepted and ready to land.
LGTM but let's make sure one of our model experts (@davidedmundson, @broulik,
@mart) gets a chance to make sure I'm not smoking crack by approving this. :
