From my recent experience of getting a similar patch reviewed, I have
the following additional feedback :-)
> + vbox.pack_start(self._status_text, padding=10)
The padding shouldn't be hardcoded to a number of pixels, see the
constants in style.py. (maybe padding=style.DEFAULT_PADDING?)
> +def get_touchpad():
> + """ Get the touchpad mode. """
> + _file_handle = open(_NODE_PATH, "r")
> + _text = _file_handle.read()
> + _file_handle.close()
Maybe put this is a try... block?
> + if touchpad == 'capacitive':
> + if path.exists(_FLAG_PATH):
> + remove(_FLAG_PATH)
> + system("echo 0 > %s" % (_NODE_PATH))
> + else:
> + _file_handle = open(_FLAG_PATH, "w")
> + _file_handle.close()
> + system("echo 1 > %s" % (_NODE_PATH))
> + return
Tomeu will nag you about "" vs. '' in the system(...) calls ;)
_______________________________________________
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel
