Review Request 111319: Move kmessagewidget to kwidgetsaddons

Romário Rios luizromario at gmail.com
Mon Jul 1 16:11:27 UTC 2013



> On July 1, 2013, 3:08 p.m., Kevin Ottens wrote:
> > tier1/kwidgetsaddons/src/kmessagewidget.cpp, line 90
> > <http://git.reviewboard.kde.org/r/111319/diff/1/?file=166799#file166799line90>
> >
> >     Could be changed to "Close this message" I guess. "Close document" has always been weird in that context after all. :-)

Yeah, I also found it a bit weird, but, since that's how it's always been, I didn't touch it.


> On July 1, 2013, 3:08 p.m., Kevin Ottens wrote:
> > tier1/kwidgetsaddons/src/kmessagewidget.cpp, line 269
> > <http://git.reviewboard.kde.org/r/111319/diff/1/?file=166799#file166799line269>
> >
> >     I think it's more readable if you keep the variable declaration as it was:
> >     QColor bg0, bg1, bg2, border, fg;
> >     
> >     It's kind of odd to have bg1 declared separately and then the other ones.

Personally, I don't like all declarations on top to then initialize the variables afterwards, and I only do this with bg1 because it's necessary in that case -- bg1 changes in the switch right below it. But, yeah, changing the coding style is also not very nice, so, reverting.


- Romário


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


On June 29, 2013, 8:24 p.m., Romário Rios wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111319/
> -----------------------------------------------------------
> 
> (Updated June 29, 2013, 8:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Moves kmessagewidget to kwidgetsaddons framework and removes all KDE dependencies from it. I don't know how to get the Close button shortcuts the same way it was gotten in kglobalactions, but I don't know if it's necessary in the case of this widget specifically.
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt ab43f17 
>   kdeui/tests/CMakeLists.txt f489b6b 
>   kdeui/tests/kmessagewidgettest.cpp eb151b9 
>   kdeui/widgets/kmessagewidget.h 9fbb176 
>   kdeui/widgets/kmessagewidget.cpp 7fb91e8 
>   tier1/kwidgetsaddons/src/CMakeLists.txt 5a37efa 
>   tier1/kwidgetsaddons/src/kmessagewidget.h PRE-CREATION 
>   tier1/kwidgetsaddons/src/kmessagewidget.cpp PRE-CREATION 
>   tier1/kwidgetsaddons/tests/CMakeLists.txt 9738c6c 
>   tier1/kwidgetsaddons/tests/kmessagewidgettest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111319/diff/
> 
> 
> Testing
> -------
> 
> Compiles, test runs fine.
> 
> 
> Thanks,
> 
> Romário Rios
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130701/d75cbd49/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list