ahmadsamir added a comment.
Ping.
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D28135
To: ahmadsamir, #plasma, davidedmundson, apol
Cc: anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack,
jraleigh, zachus, fbampaloukas, ragreen,
ahmadsamir updated this revision to Diff 78355.
ahmadsamir added a comment.
QStringList has a sort() method
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28135?vs=77979=78355
BRANCH
l-qset-fromlist (branched from master)
REVISION DETAIL
ahmadsamir added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in runnermodel.cpp:179
> But toSet() returns new container m_runners and runners are unmodified.
I don't mind reverting that bit, but it seems wasteful to me to throw
newRunners away, it is sorted and has no
anthonyfieroni added inline comments.
INLINE COMMENTS
> ahmadsamir wrote in runnermodel.cpp:179
> IIUC, the original code used toSet() to remove duplicates from both
> "m_runners" and "runners", because QSet doesn't allow duplicate items.
But toSet() returns new container m_runners and runners
ahmadsamir added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in runnermodel.cpp:179
> Here should be `m_runners = runners` to be exactly same as previous. I don't
> see much benefit of having duplicate items.
IIUC, the original code used toSet() to remove duplicates from both
anthonyfieroni added inline comments.
INLINE COMMENTS
> runnermodel.cpp:179
> +if (currRunners != newRunners) {
> +m_runners = newRunners;
>
Here should be `m_runners = runners` to be exactly same as previous. I don't
see much benefit of having duplicate items.
REPOSITORY
R120
ahmadsamir created this revision.
ahmadsamir added reviewers: Plasma, davidedmundson, apol.
Herald added a project: Plasma.
ahmadsamir requested review of this revision.
TEST PLAN
make && ctest
REPOSITORY
R120 Plasma Workspace
BRANCH
l-qset-fromlist (branched from master)
REVISION DETAIL
