davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  +1 on the concept.
  
  > Currently, the default configuration of showing all individual cores is 
pretty useless on system with a high number of cores.
  
  Agreed, it's useless.  Load gets balanced between cores so having a history 
per core tells you nothing.
  
  > This changes the default to only show all individual cores on systems with 
8 cores or less, on systems with more cores only the total cpu usage is shown
  
  If we're going to change it, I'd keep it simple and just change it, rather 
than having magic behaviour. Especially as it's just a default a user can 
enable .
  
  ---
  
  Your pattern of the JS "virtuals" is quite nice.
  
  But for isSourceValid it's a bit weird as you leave a mix two completely 
different styles.
  
  We still have the filter inside onSourceAdded / root.addSource - which 
effectively is a filter at the correct layer before we start subscribing to the 
data source.
  Then you have isValidSource applying the same filtering that we've already 
done...on top of this already filtered content.
  
  I don't understand what that's about.

REPOSITORY
  R120 Plasma Workspace

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

To: rappelman, #plasma, #vdg, davidedmundson
Cc: davidedmundson, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

Reply via email to