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

Aaron J. Seigo aseigo at kde.org
Sun May 27 18:41:08 UTC 2012


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


i'm unsure if the user should have to enter the tmp directory on the device to use to send the content. i think it would make more sense to use a "well known directory" that is unlikely to conflict on the target device such as ".kde/share/apps/plasmate/autoinstall/". it also needs to be cleared out before usage since there may already be content there.


publisher/remoteinstaller/remoteinstaller.h
<http://git.reviewboard.kde.org/r/105032/#comment11233>

    this class has one static method. just merge it into the remote installer ui.



publisher/remoteinstaller/remoteinstaller.cpp
<http://git.reviewboard.kde.org/r/105032/#comment11231>

    instead of "ip" the second parameter should more consistently be called "hostname"



publisher/remoteinstaller/remoteinstaller.cpp
<http://git.reviewboard.kde.org/r/105032/#comment11232>

    problem here, as there is not '/' separator between the execUrl and the temporaryDirectory:
    
    fish://username@hosttmpDirectory
    
    instead, use QUrl.
    
    QUrl url("fish://" + hostname);
    url.setUserName(username);
    url.setPath(temporaryDirectory);
    
    voila. properly formed url.



publisher/remoteinstaller/remoteinstaller.cpp
<http://git.reviewboard.kde.org/r/105032/#comment11234>

    this returns an asynchronous job. all the code after this line must be executed only after the job succeeds (or not, if the job fails, of course).
    
    some user feedback as to what the cause of the error is would be useful.


- Aaron J. Seigo


On May 24, 2012, 5:49 p.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105032/
> -----------------------------------------------------------
> 
> (Updated May 24, 2012, 5:49 p.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 7e1bf4b 
>   publisher/publisher.h ee57cf3 
>   publisher/publisher.cpp 11ddf36 
>   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/20120527/98613034/attachment-0001.html>


More information about the Plasma-devel mailing list