Thanks to Andrii's help, the code is now statically initializing the generic
methods. Now there shouldn't be any perf impact.
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:
On 2015/06/09 09:18:01, tandrii(chromium) wrote:
inherit from "(object)"
Done.
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode57
tools/testrunner/local/progress.py:57: def register(self, indicator):
On 2015/06/09 09:18:01, tandrii(chromium) wrote:
nit: maybe Register for consistency with code around?
Done.
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode61
tools/testrunner/local/progress.py:61: """Generic event."""
On 2015/06/09 09:18:01, tandrii(chromium) wrote:
how about: """Generic event dispatcher.""" ?
and I'd add an assert:
assert callable(ProgressIndicator.__dict__[func_name]), "unknown
event: %s" %
func_name
now changed design...
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)
On 2015/06/09 09:18:01, tandrii(chromium) wrote:
maybe also add setattr(self, func_name, func) ?
now changed design...
https://codereview.chromium.org/1171943002/diff/1/tools/testrunner/local/progress.py#newcode70
tools/testrunner/local/progress.py:70: def SetRunner(self, runner):
On 2015/06/09 10:14:34, tandrii(chromium) wrote:
On 2015/06/09 09:49:51, Jakob wrote:
> On 2015/06/09 09:18:01, tandrii(chromium) wrote:
> > IMO, I'd keep the constructor.
>
> Agreed. The constructor exists mostly for documentation purposes too
-- the
idea
> is that looking at the constructor is enough to understand the
object layout,
as
> all fields are initialized there.
>
> > Am I right that SetRunner is really for documentation, not really
for
anything
> > else? because bla.runner = runner still works anyways...
>
> I guess unless setattr is implemented as you suggested above,
exposing a
setter
> is the only [elegant] way to set the runner for all registered
progress
> indicators.
I don't think setattr above will change anything, as it only binds
functions.
Setters are not pythonic, properties are. Yet i'm guilty of using them
myself,
purely for being more explicit = e.g., documentation. So, don't mind
having it
SetRunner() here :)
I keep the constructor, but I also need the setter to not be required to
make also attribute assignment generic in the IndicatorNotifier (if I
add a generic __setattr__ it also collides with the initialization of
self.indicators = []).
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.
