araujoluis marked an inline comment as done.
araujoluis added inline comments.

INLINE COMMENTS

> patrickelectric wrote in kcolorcombo.cpp:74
> Missing const, also the name should be `innerColor` with a capital C if we 
> are following the code style from this file.

Done!

> patrickelectric wrote in kcolorcombo.cpp:83
> missing reference for innercolor ?

Yes, reference not found for innercolor or innerColor

> patrickelectric wrote in kcolorcombo.cpp:107
> It's missing const

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> Maybe changing the logic to make it more simpler:
> 
>   QRect colorRect = tv.toString().isEmpty() ? innerrect : 
> QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, 
> innerrect.width(), innerrect.height() / 2);

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> Maybe changing the logic to make it more simpler:
> 
>   QRect colorRect = tv.toString().isEmpty() ? innerrect : 
> QRect(innerrect.x(), innerrect.y() + innerrect.height() / 2, 
> innerrect.width(), innerrect.height() / 2);

Done!

> patrickelectric wrote in kcolorcombo.cpp:107
> It's missing const

Done!

> cfeck wrote in kcolorcombo.cpp:277
> Variables declared in 'for' are local, so just use 'color'.

Done!

> cfeck wrote in kcolorcombo.h:91
> typos: of; selection

Done!

> cfeck wrote in kcolorcombo.h:110
> typo: selection

Done!

> cfeck wrote in kcolorcombo.h:111
> This sentence is confusing. I guess you wanted to write "if there are no 
> named colors, the returned list is empty." (to clarify that it won't return 
> the standard list).

Done!

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D29502

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham
Cc: broulik, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

Reply via email to