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