Make RGB overview use colors from skin#508
Conversation
There was a problem hiding this comment.
please use c++-style cast (static_cast())
|
Comments addressed. |
There was a problem hiding this comment.
don't align the equal signs, please.
There was a problem hiding this comment.
Somehow, this seems nicer (at least to me). 😕
There was a problem hiding this comment.
I won't be strict about it, but this type of alignment is not part of our style guide and it doesn't really improve readability. Subsequent work can end up requiring changes to whitespace that aren't even relevant to the code being changed, making it harder to review. (see https://github.com/mixxxdj/mixxx/blob/master/src/vinylcontrol/vinylcontrol.cpp for what happens when this runs amok)
There was a problem hiding this comment.
Yes, I can see. Good point.
There was a problem hiding this comment.
Btw, I noticed a similar problem in https://github.com/mixxxdj/mixxx/blob/master/src/waveform/renderers/glwaveformrendererrgb.cpp#L105. Do you want me to fix this as well?
There was a problem hiding this comment.
if you're not touching the code in this PR, don't worry about it
* Move declaration of some variables to the point of assignment. * Remove horizontal alignment in some lines.
|
LGTM thanks! |
Make RGB overview use colors from skin
|
btw have you signed our contributor agreement? |
|
Yes, when I first contributed Mixxx (with RGB waveform renderer - #118). |
2 participants
This makes RGB overview use colors defined by
<SignalRGB*Color>tags in skin, as discussed on: