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