GNOME Bugzilla – Bug 777216
PEP 8 fix : __init__.py
Last modified: 2017-01-16 09:59:32 UTC
Currently __init__.py does not follow all PEP guidelines.
Created attachment 343432 [details] [review] PEP fixes
I believe singleton pattern is implemented with the TrackerWrapper. The current usage is for example in window.py :> tracker = TrackerWrapper().tracker Should I try to get rid of the constructor to make it look like this? tracker = TrackerWrapper.tracker It might require a meta class implementation.
Another idea would be to make it a module variable, like it was done with Grilo. In the end of the grilo file, grilo = Grilo() And then all other classes simply do from gnomemusic.grilo, import grilo So this will become like from gnomemusic, import tracker
Review of attachment 343432 [details] [review]: Some minor issues noted, but looking good. ::: gnomemusic/__init__.py @@ +42,3 @@ + Decorate functions with this to show their runtime info. + Use with the debug flag. + """ sphinx style, see other recent docstrings @@ +56,3 @@ if 'rateLimitedFunction' not in name: logger.debug("%s%s(%s)[%s:%s]", + '|' * tabbing, name, params, filename, lineno) As much as possible on one line, then break on the comma to the next. @@ +66,2 @@ elapsed_time = ', took %02f' % elapsed if elapsed_time or retval is not None: multi statement if's in () with the operator on a new line up front eg. if (cond1 and cond2): @@ +68,3 @@ if 'rateLimitedFunction' not in name: + logger.debug("%s returned %s%s", + '|' * tabbing, repr(retval), elapsed_time) indentation @@ +88,3 @@ return repr(self) + + __instance = None _instance is fine
Regarding comments 2 & 3, it can stay as it is for now.
Created attachment 343449 [details] [review] PEP 8 fixes and added docstrings
Made some minor enhancements and rewrote the commit message a bit. Thanks for the patch.