Review Request: populate the settings of the rssnow-qml plasmoid

[Marco Martin]

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


on the right track, there are still some issues to iron out


rssnow/package/contents/ui/main.qml
<http://git.reviewboard.kde.org/r/106582/#comment16231>

    exporting minimumWidth/minimumHeight here was correct, hybrid ones can too



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16225>

    various rrsnow-qml can become jusr rssnow



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16226>

    no need to put this here, the qml root element can export minimumWIdth/minimumHeight properties



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16229>

    it's a function outside createconfigurationinterface that accesses the configuration ui: check for existence of the pointers you are touching



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16230>

    dangerous here too, check the pointers



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16227>

    never call slots emitFoo
    they should have names in the past, like fooHappened, fooChanged etc
    
    this should reflect what it does, eg
    void feedAdded(const QString &feed)



rssnow/rssnow.cpp
<http://git.reviewboard.kde.org/r/106582/#comment16228>

    this should be:
    void busyChanged(bool busy)
    {
    setBusy(busy)
    }


- Marco Martin


On Sept. 26, 2012, 3:23 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106582/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2012, 3:23 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> I have populated the settings of the rssnow-qml plasmoid
> and they are identical to the ones of the c++ one.
> 
> In order to do that I made the rssnow-qml plasmoid into a hybrid.
> 
> After submit of this review I believe that it is ready to be
> moved into the kdeplasma-addons and to remove the c++ one.
> (I would prefer to open a new review for the move)
> 
> Also I have pushed the latest changes(this diff) into the
> terietor/rssnow/settings branch in declarative-plasmoids
> 
> 
> Diffs
> -----
> 
>   rssnow/CMakeLists.txt 79b3581 
>   rssnow/feedsConfig.ui PRE-CREATION 
>   rssnow/generalConfig.ui PRE-CREATION 
>   rssnow/package/contents/ui/DropRssEntry.qml PRE-CREATION 
>   rssnow/package/contents/ui/ListItemEntry.qml 657b14b 
>   rssnow/package/contents/ui/config.ui e1e3409 
>   rssnow/package/contents/ui/main.qml a575c29 
>   rssnow/plasma-applet-rssnow2.desktop PRE-CREATION 
>   rssnow/rssnow.h PRE-CREATION 
>   rssnow/rssnow.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106582/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121018/8e311ed5/attachment.html>