-----------------------------------------------------------
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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to