[cr tool] Fixing import problems

[cr tool] Fixing import problems

BUG=327999
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243952

[Miguel Garcia, 2013-12-16 14:23:08]

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py
File tools/cr/cr/actions/gdb.py (right):

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py#n...
tools/cr/cr/actions/gdb.py:37: @classmethod
I would not make this a class method since it's called in the constructor,
instead make it a normal method and pass DETECTED as an arg?

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/ninja.py
File tools/cr/cr/actions/ninja.py (left):

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/ninja.py...
tools/cr/cr/actions/ninja.py:79: @classmethod
ditto

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/commands/sync.py
File tools/cr/cr/commands/sync.py (right):

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/commands/sync.py...
tools/cr/cr/commands/sync.py:48: @classmethod
same here

[ian_cottrell, 2013-12-16 17:40:34]

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py
File tools/cr/cr/actions/gdb.py (right):

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py#n...
tools/cr/cr/actions/gdb.py:37: @classmethod
On 2013/12/16 14:23:09, Miguel Garcia wrote:
> I would not make this a class method since it's called in the constructor,
> instead make it a normal method and pass DETECTED as an arg?

It does not need access to an instance of the class, it is just hooked to the
constructor because it's a suitably delayed place where we know the dependencies
are ready, but before the instance could possibly be used.
It really ought to be called only on the *first* constructor invocation, but
that was more work than it was worth, as only one of these is ever built.
Alternatively, I could hook into the GetInstance call that is the only thing
that ever calls the constructor, and is also a class method?

[Miguel Garcia, 2013-12-16 17:43:58]

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py
File tools/cr/cr/actions/gdb.py (right):

https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py#n...
tools/cr/cr/actions/gdb.py:37: @classmethod
On 2013/12/16 17:40:34, ian_cottrell wrote:
> On 2013/12/16 14:23:09, Miguel Garcia wrote:
> > I would not make this a class method since it's called in the constructor,
> > instead make it a normal method and pass DETECTED as an arg?
> 
> It does not need access to an instance of the class, it is just hooked to the
> constructor because it's a suitably delayed place where we know the
dependencies
> are ready, but before the instance could possibly be used.
> It really ought to be called only on the *first* constructor invocation, but
> that was more work than it was worth, as only one of these is ever built.
> Alternatively, I could hook into the GetInstance call that is the only thing
> that ever calls the constructor, and is also a class method?

Making it part instead of the constructor does seem cleaner

[ian_cottrell, 2014-01-09 17:23:01]

On 2013/12/16 17:43:58, Miguel Garcia wrote:
> https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py
> File tools/cr/cr/actions/gdb.py (right):
> 
>
https://codereview.chromium.org/116363002/diff/1/tools/cr/cr/actions/gdb.py#n...
> tools/cr/cr/actions/gdb.py:37: @classmethod
> On 2013/12/16 17:40:34, ian_cottrell wrote:
> > On 2013/12/16 14:23:09, Miguel Garcia wrote:
> > > I would not make this a class method since it's called in the constructor,
> > > instead make it a normal method and pass DETECTED as an arg?
> > 
> > It does not need access to an instance of the class, it is just hooked to
the
> > constructor because it's a suitably delayed place where we know the
> dependencies
> > are ready, but before the instance could possibly be used.
> > It really ought to be called only on the *first* constructor invocation, but
> > that was more work than it was worth, as only one of these is ever built.
> > Alternatively, I could hook into the GetInstance call that is the only thing
> > that ever calls the constructor, and is also a class method?
> 
> Making it part instead of the constructor does seem cleaner

I have added a hook to the plugin system for delayed class initialization, and
moved the code into that.

[rmcilroy, 2014-01-09 17:40:42]

I like this better.

lgtm

[commit-bot: I haz the power, 2014-01-09 17:50:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/iancottrell@chromium.org/116363002/20001

[commit-bot: I haz the power, 2014-01-09 19:29:38]

Change committed as 243952