Review Request 108212: [1/3] Refactor EnvironmentSelectionWidget.

[Milian Wolff]

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



plugins/execute/nativeappconfig.cpp
<http://git.reviewboard.kde.org/r/108212/#comment32294>

    QString() instead of "" please.



plugins/executescript/scriptappconfig.cpp
<http://git.reviewboard.kde.org/r/108212/#comment32295>

    same here



util/environmentselectionmodel.h
<http://git.reviewboard.kde.org/r/108212/#comment32298>

    you should probably also implement header data and give proper titles to the columns



util/environmentselectionmodel.h
<http://git.reviewboard.kde.org/r/108212/#comment32296>

    as I said before, don't return references



util/environmentselectionwidget.h
<http://git.reviewboard.kde.org/r/108212/#comment32299>

    as before, no references please



util/environmentselectionwidget.cpp
<http://git.reviewboard.kde.org/r/108212/#comment32300>

    that looks wrong



util/environmentselectionwidget.cpp
<http://git.reviewboard.kde.org/r/108212/#comment32301>

    indentation wrong, please put the : on a new line and indent the members a level as well. The commas should also lead the new lines:
    
    Foo()
      : a()
      , b()
      , ...
    {
    }


- Milian Wolff


On Dec. 4, 2013, 4:02 p.m., Ivan Shapovalov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108212/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2013, 4:02 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Feature-wise this gets rid of dependency on default KConfigXT value being "default" and allows explicitly specifying to use a default environment group whatever it will be.
> 
> Now an empty value shall be used by default, which has meaning of "use whatever is set by default at runtime".
> Also, a special "Use default" line is now always added to the combo-box which maps to the empty config value, so user won't see empty entries in the combo-box.
> 
> 
> Diffs
> -----
> 
>   plugins/execute/nativeappconfig.cpp 6f2dd8c 
>   plugins/executescript/scriptappconfig.cpp 370231a 
>   util/CMakeLists.txt 87384bd 
>   util/environmentconfigurebutton.cpp b95e536 
>   util/environmentselectionmodel.h PRE-CREATION 
>   util/environmentselectionmodel.cpp PRE-CREATION 
>   util/environmentselectionwidget.h 0784ba1 
>   util/environmentselectionwidget.cpp a198f32 
> 
> Diff: http://git.reviewboard.kde.org/r/108212/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Ivan Shapovalov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20131205/738e4a86/attachment-0001.html>