D5584: Introduction of runtimes

[Aleix Pol Gonzalez]

apol marked 23 inline comments as done.
apol added inline comments.

INLINE COMMENTS

> mwolff wrote in iruntime.h:84
> how is this supposed to be used? the API isn't straight-forward, i.e. why is this protected? Why do I call it on this class instead of something like `controller->setEnabled(theRuntime)`?

See DockerRuntime::setEnabled. There's some things we need to get ready before getting it announced as enabled.

> mwolff wrote in iruntimecontroller.h:52
> do we usually add multiple ones in a go? I'd expect when the user adds _a_ runtime, it's only one - so having an add function that does not take a vector would be good to have.
> 
> also, are we never removing runtimes?

Yes, you are right, I added this in an iteration where I had a model. Probably doesn't make much sense now.

Some plugins add at bulk, such as docker at boot.

Will remove them on deletion.

> mwolff wrote in outputexecutejob.cpp:109
> out of interest: why is this required? what state is the process in?

I'll revert it and debug again, I don't remember why I added it anymore.

> mwolff wrote in dockerplugin.cpp:75
> if you'd make this whole function a lambda and inline it above into the connect, you could capture the process directly, removing the need to call sender()

meh.

> mwolff wrote in dockerplugin.cpp:88
> so the runtime controller takes ownership?

Yes.

> mwolff wrote in dockerruntime.cpp:72
> qgetenv(USER)? Don't we have KDE api for this? I doubt this would work on windows, but then again - most of the commands here don't work on windows. make sure this whole plugin isn't compiled on windows?

I looked into KUser, I don't see how to create a user for the current user. Something to look into.

> mwolff wrote in flatpakruntime.h:40
> why both K and Q process?

KProcess p;
  p.setProgram({a, b, c});
  p->QProcess::start();

^this doesn't do what it should do.

REPOSITORY
  R33 KDevPlatform

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

To: apol, #kdevelop, mwolff
Cc: mwolff, anthonyfieroni, kossebau, geetamc, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170514/54e87fff/attachment.html>