lgtm
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#newcode70
tools/testrunner/local/progress.py:70: def SetRunner(self, runner):
On 2015/06/09 12:47:14, Michael Achenbach wrote:
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 = []).
Acknowledged.
https://codereview.chromium.org/1171943002/diff/20001/tools/testrunner/local/progress.py
File tools/testrunner/local/progress.py (right):
https://codereview.chromium.org/1171943002/diff/20001/tools/testrunner/local/progress.py#newcode96
tools/testrunner/local/progress.py:96: def _Indicator_apply(func):
nit: then maybe _IndicatorNotifier_apply, and even better move its body
inside the loop, but make sure to keep func.__name__ instead of
func_name to avoid closure by ref problem.
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.
