[Bugs] #2141 UNSP: Memory and CPU status indicator for the frame.

[Sugar Labs Bugs]

#2141: Memory and CPU status indicator for the frame.
------------------------------------------+---------------------------------
    Reporter:  m_anish                    |          Owner:  tomeu                      
        Type:  enhancement                |         Status:  new                        
    Priority:  Unspecified by Maintainer  |      Milestone:  Unspecified by Release Team
   Component:  sugar                      |        Version:  Unspecified                
    Severity:  Unspecified                |       Keywords:  r! dextrose                
Distribution:  Unspecified                |   Status_field:  Unconfirmed                
------------------------------------------+---------------------------------
Changes (by tomeu):

  * keywords:  r?, dextrose => r! dextrose


Comment:

 > +        self.create_palette()

 Why create the palette at this point?

 > +        logging.debug('palette created')

 Might not be useful any more.

 > +        self.palette.connect('system-mood-changed',
 > +                self._system_mood_changed_cb)

 Looks like the Palette is doing the role of the model, which is weird.
 Instead, we should have a Model class and both the icon and the palette
 would listen to it.

 > +    def _system_mood_changed_cb(self, gobject, mood):

 Two leading underscores.

 > +        self.vbox = gtk.VBox()

 All members should be private unless they are needed outside its class.

 > +        return [int(count)
 > +           for count in file('/proc/stat').readline().split()[1:6]]

 Can we get this in separate lines with meaningfully-named intermediate
 variables?

 > +        _cpu_times_current = [(new - old)
 > +            for new, old in zip(_cpu_times_new, self._cpu_times)]

 Same here.

 > +            self._cpu_text.set_label(_('CPU in use: %d%%' %
 cpu_in_use))

 The _() should apply only to the string to be translated, see for other
 instances of this in the patch.

 > +            self._cpu_bar.set_fraction( cpu_in_use / 100.0 )

 Extra spaces inside the parens, look for other instances of this.

 > +            # both cpu_free and memory_free lie between 0-100
 > +            system_mood = _SYSTEM_MOODS[
 > +                    int( ( 200 - (cpu_in_use + memory_in_use) ) / 66.66
 ) ]

 Is it really better than a few simple elifs? You would also avoid the
 dangerous except AttributeError: below.

 > +    def _stop_computing_statistics(self):
 > +        """
 > +        Stop computing usage statistics and display and error message
 > +        since we've hit an exception.

 Seems like here we are just displaying the error message?

 > +        self.emit('system-mood-changed', '-error')

 May be better to have the moods be constants so we don't duplicate the
 strings?

-- 
Ticket URL: <http://bugs.sugarlabs.org/ticket/2141#comment:2>
Sugar Labs <http://sugarlabs.org/>
Sugar Labs bug tracking system