Review Request: Plasmate: fix publisher's combobox and doCMake slot

Giorgos Tsiapaliwkas terietor at gmail.com
Wed May 23 18:23:29 UTC 2012



> On May 22, 2012, 11:22 p.m., Sebastian Kügler wrote:
> > Looks like a sensible change, otherwise. If you've fixed the issues aseigo notes, please go ahead and merge into master.
> > 
> > In the same part, there's an interaction problem, however:
> > Assumption: installing an app is quite a common thing during development (correct me if you think otherwise)
> > Right now, there is no button to trigger installation with the currently selected mode, so one has to switch the combo to empty, then to the preferred installation method. A button "Install!" would fix that.
> > Also, doing the installtion when the combobox changes is quite uncommon, as it is not explicit that the installation actually already happens when you choose something. This should move into a button, which can conveniently go right next to the combo, and then the combo changed to not install on indexChanged. Also, it would make sense to add an action andshortcut for installation to the toolbar above.
> > 
> > This is not an issue with this patch, just how we should improve that part of the UI. I used it for a development task yesterday, and this installatio workflow issue struck me as overly complex and "have to learn it" rather than "understand it without thinking".
> >

>Right now, there is no button to trigger installation with the currently selected mode, so one has to switch the combo to empty, then to the preferred installation method. A button "Install!" would fix that.
>Also, doing the installtion when the combobox changes is quite uncommon, as it is not explicit that the installation actually already happens when you choose something. This should move into a button, which can conveniently >go right next to the combo, and then the combo changed to not install on indexChanged. 

I agree. I will update the diff tomorrow with the new button if we all agree with the new button.

>Also, it would make sense to add an action andshortcut for installation to the toolbar above.
I guess it would be a good idea to add an action in the toolbar. What do you think?

>Also, it would make sense to add an action andshortcut for installation to the toolbar above.
Which shortcut do you want?

I would prefer to use the plasmapkg and not cmake.
Everyone is using plasmapkg and it is safer (from the POV of a future bug in cmake) due to the fact the more developers use it.


- Giorgos


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


On May 16, 2012, 6:38 p.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104969/
> -----------------------------------------------------------
> 
> (Updated May 16, 2012, 6:38 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Hello,
> 
> those are some issues which plasmate's publisher has
> 
> problem 1: the publisher's combobox wasn't aware for the right slot. When currentIndex was emitted both slots(doPlasmaPkg and doCMake was called.) I fixed that.
> problem 2: publisher's cmake process was trying to install a projectName.plasmoid file and not projectPath/CMakeLists.txt. I fixed that.
> problem 3: when the cmake slot is called, cmake creates the known temporary files in directory like ~/.kde4/share/apps/plasmate/projectName. I haven't fixed that.
> How can I change the current directory with Qt?
> 
> 
> Diffs
> -----
> 
>   publisher/publisher.h 6eba693 
>   publisher/publisher.cpp 3fcd268 
> 
> Diff: http://git.reviewboard.kde.org/r/104969/diff/
> 
> 
> Testing
> -------
> 
> WIP
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliwkas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120523/6f23f3a7/attachment-0001.html>


More information about the Plasma-devel mailing list