davidedmundson added subscribers: adridg, vonreth.
davidedmundson added a comment.
Awesome progress. Thanks ever so much.
@vonreth could you test the windows side for us and tell us how to do the
unit test?
@adridg could you run this on BSD. You should just need to run
./bin/kprocesslisttest and see if it passes.
INLINE COMMENTS
> kprocesslist.cpp:38
> +KProcessInfoPrivate::KProcessInfoPrivate(KProcessInfo* q) :
> + q_ptr(q),
> + valid(false),
we don't need a q_ptr
It's only useful if the private object needs to signal things upwards.
Our data structure is static.
> kprocesslist.cpp:65
> +{
> + d_ptr->valid = true;
> + d_ptr->pid = other.pid();
x = ProcessInfo()
x.valid() == false;
y=x;
y.valid() == true;
But you can simplify all this and the other one to
*d_ptr = *other.d_ptr;
> kprocesslist.cpp:72
> +KProcessInfo::~KProcessInfo()
> +{
> +}
delete d_ptr;
OR
replace make d_ptr a QScopedPointer
OR
make it a QSharedDataPointer and our KProcessInfoPrivate inherit from
QSharedData
(which is a bit more work but my favourite option)
> kprocesslist.h:51
> + KProcessInfo();
> + KProcessInfo(qint64 pid, QString name, QString user);
> + KProcessInfo(const KProcessInfo &other);
qint64 pid, const QString &name, const QString &user
it saves a string shallow copy
> kprocesslist_p.h:48-49
> + QString name;
> + QString image;
> + QString state;
> + QString user;
We should probably either kill these 2 or use them.
REPOSITORY
R244 KCoreAddons
REVISION DETAIL
https://phabricator.kde.org/D20007
To: hallas, davidedmundson, broulik
Cc: vonreth, adridg, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham,
bruns
