Review Request 129884: Fix for bug 360539

[Tomaz Canabrava]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129884/#review102247
-----------------------------------------------------------



I know that this was not accepted because of the other review that exists, but I think it's worth the review, please take a look at my comments for your *next* trial.
also, use phabricator (with the arc diff command) instead of reviewboard.


sublime/idealdockwidget.cpp (line 84)
<https://git.reviewboard.kde.org/r/129884/#comment68254>

    This a Expensive call, it will go thru *all* children of the widget() and test for QToolBars, findChild should be the last option.



sublime/idealdockwidget.cpp (line 86)
<https://git.reviewboard.kde.org/r/129884/#comment68255>

    m_view->document()->title() seems to be the title of the document, not the toolbar, so if you opened two or three tolbars, this would work?
    
    also, the string "Toolbar for %1" seems strange in a Configuration file (usually there are no spaces)



sublime/idealdockwidget.cpp (line 87)
<https://git.reviewboard.kde.org/r/129884/#comment68256>

    Qt4 style connects shouldn't be used, because they could have errors that are not checked at build time.



sublime/idealdockwidget.cpp (line 89)
<https://git.reviewboard.kde.org/r/129884/#comment68257>

    Lots of spaces not removed on the code after you saved your files.


- Tomaz  Canabrava


On Jan. 25, 2017, 7:28 p.m., Damir Porobic wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129884/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2017, 7:28 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Bugs: 360539
>     http://bugs.kde.org/show_bug.cgi?id=360539
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Fix for bug 360539. Saving visibility status of view toolbars
> 
> 
> Diffs
> -----
> 
>   sublime/idealdockwidget.h c966481 
>   sublime/idealdockwidget.cpp c79b1ba 
> 
> Diff: https://git.reviewboard.kde.org/r/129884/diff/
> 
> 
> Testing
> -------
> 
> Tested if toolbars are correctly hidden and shown again after closing and opening up kdevelop. 
> Checked config kdeveloprc to confirm that variables are written corretly
> 
> 
> Thanks,
> 
> Damir Porobic
> 
>

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