Review Request: Changes at the functionality of the delete button.

[Sebastian Kügler]

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



projectmanager/projectmanager.h
<http://git.reviewboard.kde.org/r/101535/#comment3206>

    m_destroyFlag seems to be used only internally, in that case you don't need to add an accessor, just refer to m_destroyFlag directly in your code



projectmanager/projectmanager.h
<http://git.reviewboard.kde.org/r/101535/#comment3204>

    Should use KPushButton, just like its siblings (we generally prefer the K* classes above the Q* classes, since they usually add integration features)



projectmanager/projectmanager.cpp
<http://git.reviewboard.kde.org/r/101535/#comment3201>

    The usability of this dialog can be better if you put the actions that will follow on the button, i.e. 
    
    [remove] [cancel]
    
    THat way, a user can see what she's doing without reading the full dialog, and scanning for "not", "no" and so on.



projectmanager/projectmanager.cpp
<http://git.reviewboard.kde.org/r/101535/#comment3202>

    this guy can be const (as you're not planning to change it afterwards, it will also be a tiny bit more efficient.
    
    Text should read: i18n("Are you sure you want to delete the project? This cannot be undone.");



projectmanager/projectmanager.cpp
<http://git.reviewboard.kde.org/r/101535/#comment3203>

    please enclose with parentheses and put it on different lines:
    
    if (bla) {
       // foo
    }



projectmanager/projectmanager.cpp
<http://git.reviewboard.kde.org/r/101535/#comment3205>

    add a space here:
    
    if (bla) ... instead of if(bla)


- Sebastian


On June 10, 2011, 9 p.m., Giorgos Tsiapaliwkas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101535/
> -----------------------------------------------------------
> 
> (Updated June 10, 2011, 9 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Hello,
> 
> after a discussion with Aaron Seigo at the #plasma we decided to add a new button at the project manager which is named "Remove Project From List",which deletes the project only from the list(not from disk).The delete button was renamed to "Remove Project From Disk".Also i consider it properly to change the names of some function in order to be more relative with the buttons names.As well i changed and the functionality of confirmdeletion() in order to avoid using the same code twice.
> 
> 
> Diffs
> -----
> 
>   projectmanager/projectmanager.h 2c5bff2 
>   projectmanager/projectmanager.cpp c00fd27 
> 
> Diff: http://git.reviewboard.kde.org/r/101535/diff
> 
> 
> Testing
> -------
> 
> compiles and runs without regressions.
> tested by me.
> 
> 
> Thanks,
> 
> Giorgos
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110616/af4de3d4/attachment.htm