Review Request: Plasmate: request to merge terietor/remoteinstaller to master

[David Edmundson]

> On June 2, 2012, 3:14 p.m., David Edmundson wrote:
> > publisher/remoteinstaller/remoteinstallerdialog.cpp, line 53
> > <http://git.reviewboard.kde.org/r/105032/diff/3/?file=66057#file66057line53>
> >
> >     This looks like it leaks.
> >     
> >     QScopedPointer perhaps?
> 
> Giorgos Tsiapaliwkas wrote:
>     Here http://blogs.kde.org/node/3919 it says to use QPointer. Anyway the code was removed

QPointer is good, however you then have to "delete dialog" at the end of the method. Which you've done elsewhere and is as shown in the link above.


> On June 2, 2012, 3:14 p.m., David Edmundson wrote:
> > publisher/remoteinstaller/remoteinstallerdialog.cpp, line 80
> > <http://git.reviewboard.kde.org/r/105032/diff/3/?file=66057#file66057line80>
> >
> >     Why is this needed? When you create m_installer, you pass this as the parent. 
> >
> 
> Giorgos Tsiapaliwkas wrote:
>     Are you sure that I pass this as a parent?
>     
>     this is the ctor,
>     
>     RemoteInstaller(const QString& username, const QString& hostname, const QString& source, QWidget *window, QObject* parent = 0);
>     
>     I didn't like the m_installer = new RemoteInstaller(m_username, m_hostname, m_packagePath, this, this);
>     so I cast(with qobject_cast) the parent into a m_widget;
>     
>     I removed delete m_installer and the dtor
>

Last argument to m_installer is the parent, therefore when "this" (our instance of RemoteInstallerDialog) gets deleted our instance m_installer will be deleted also, and we don't need to do it explicitly.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105032/#review14379
-----------------------------------------------------------


On June 3, 2012, 9:32 a.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105032/
> -----------------------------------------------------------
> 
> (Updated June 3, 2012, 9:32 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Hello,
> 
> in terietor/remoteinstaller I have create a remote installer.
> The goal is to install the projects remotely to another computer.
> 
> You access the remote install either inside plasmate or using the plasmaremoteinstaller binary.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 12f8a3a 
>   publisher/publisher.h 5f40ae0 
>   publisher/publisher.cpp fd87364 
>   publisher/remoteinstaller/remoteinstaller.h PRE-CREATION 
>   publisher/remoteinstaller/remoteinstaller.cpp PRE-CREATION 
>   publisher/remoteinstaller/remoteinstaller.ui PRE-CREATION 
>   publisher/remoteinstaller/remoteinstallerdialog.h PRE-CREATION 
>   publisher/remoteinstaller/remoteinstallerdialog.cpp PRE-CREATION 
>   publisher/remoteinstaller/standalone/main.cpp PRE-CREATION 
>   publisher/remoteinstaller/standalone/plasmaremoteinstaller.h PRE-CREATION 
>   publisher/remoteinstaller/standalone/plasmaremoteinstaller.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105032/diff/
> 
> 
> Testing
> -------
> 
> I couldn't find any bugs in the remote installer, but the proccess fails during KIO::copy.
> Apparently I am not writing correct something in the fish protocol.
> Right now the remote installer tries to do the above
> 
> KIO::copy("/home/terietor/.kde4/share/apps/plasmate/project, "fish://user@ip/home/user")
> 
> but it fails
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliwkas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120603/e31f925b/attachment.html>