https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py
File tools/testrunner/local/progress.py (right):
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode52
tools/testrunner/local/progress.py:52: class IndicatorNotifier:
inherit from "(object)"
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode57
tools/testrunner/local/progress.py:57: def register(self, indicator):
nit: maybe Register for consistency with code around?
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode61
tools/testrunner/local/progress.py:61: """Generic event."""
how about: """Generic event dispatcher.""" ?
and I'd add an assert:
assert callable(ProgressIndicator.__dict__[func_name]), "unknown event:
%s" % func_name
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode64
tools/testrunner/local/progress.py:64: getattr(indicator,
func_name)(*args, **kwargs)
maybe also add setattr(self, func_name, func) ?
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode70
tools/testrunner/local/progress.py:70: def SetRunner(self, runner):
IMO, I'd keep the constructor.
Am I right that SetRunner is really for documentation, not really for
anything else? because bla.runner = runner still works anyways...
https://codereview.chromium.org/1171943002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
