Review Request 129884: Fix for bug 360539

Tomaz Canabrava tcanabrava at kde.org
Thu Jan 26 12:21:00 UTC 2017



> On Jan. 26, 2017, 9:58 a.m., Tomaz  Canabrava wrote:
> > 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.
> 
> Damir Porobic wrote:
>     Thanks for providing the feedback, that helps better understand the code. Is it worth reworking this as you have obviously provided a solution for this already and I don't wont to bind resources just for teaching me. 
>     I was using reviewboard because the KDE handbook said so. Should I skip it and go directly to phabricator (-> create diff)?

There's the Phabricator command line tools, you can create a review there using the command 'arc diff'


> On Jan. 26, 2017, 9:58 a.m., Tomaz  Canabrava wrote:
> > sublime/idealdockwidget.cpp, line 87
> > <https://git.reviewboard.kde.org/r/129884/diff/1/?file=490167#file490167line87>
> >
> >     Qt4 style connects shouldn't be used, because they could have errors that are not checked at build time.
> 
> Damir Porobic wrote:
>     Was not aware of that, I should be using something like this?
>     connect(toolBar,visibilityChanged(bool),&QAction::toggled,this,toolbarVisibilityChange(bool));

almost.
connect(Action, &QAction::triggered,  this, &ClassName::ToolbarVisibilityChange);


- Tomaz


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


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/f2fe6203/attachment.html>


More information about the KDevelop-devel mailing list