rjvbb added a comment.

  > - please re-add the screenshot from Review Board. (sorry I was not aware of 
this review request cause I was not in the list of reviewers, even though 
official maintainer of oxygen ...)
  
  Sorry about that, I thought you'd be a member of the Plasma group. But I had 
a hunch so added a few reviewers manually this time.
  
  > - right to left layout action <- no. There is already one at the bottom of 
the window.
  
  The goal was not to duplicate functionality, but to add an additional item to 
the Layout menu that was not mutually exclusive with the others, as different 
styles can render such items differently. It also gives the possibility to add 
a separator.
  IIRC the patch *would* become somewhat simpler if I just leave the menu item 
but make it a noop. Would you be OK with that?
  
  > - this review should really be several, one per feature: one for the 
checkboxes/radiobuttons in the mdi window, one for the colorschemechooser. Can 
you split ?
  
  Yay, extra work... Fortunately not too much, will do later today.
  
  > - finally, there is need for more detail review (once above is done). For 
instance, in ColorSchemeChooser you use SUPPORT_THEME_SAVING, but this one is 
set/defined nowhere.
  
  TBH, I was more or less hoping for feedback like this. I also don't see the 
need to save this setting but didn't want to strip it out immediately, also out 
of some form of respect for Alexander's original work.

REPOSITORY
  R113 Oxygen Theme

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

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

Reply via email to