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

[Aaron J. Seigo]

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


getting closer :)


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

    this code is duplicated by different install methods in this class. as it includes string literals for config keys and groups, this makes it brittle for long term maintenance.
    
    consider moving it to a separate private method in this class e.g. QString currentPackagePath() const



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

    this is an implementation detail of RemoteInstaller and as such it should be encapsulated in that class, not here.
    
    in fact, it looks like the metadata.desktop filename gets dropped in RemoteInstaller anyway.
    
    so this should be made to work without metadata.desktop being appended.



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

    const bool ok



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

    on error, this should probably stop the process, preventing doInstall() from being called.
    
    this is most easily done by making doInstall private, calling doInstall at the end of the constructor and returning early from the constructor when the temp url creation fails.



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

    this error needs to be caught in doPlasmaPkg. it is an async job, so the error may not happen until doPlasmaPkg.



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

    doInstall should simply be called from the RemoteInstall object.


- Aaron J. Seigo


On June 12, 2012, 12:10 p.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105032/
> -----------------------------------------------------------
> 
> (Updated June 12, 2012, 12:10 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 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/20120614/a2ff6d9b/attachment-0001.html>