Docserver: Add more support for object level availability in templates

Docserver: Add more support for object level availability in templates

BUG=233982
NOTRY=True

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

[ahernandez, 2014-07-02 21:33:06]

I've added in support for properties, events, and types. PTAL.

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/availability_finder.py (left):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/availability_finder.py:29: def
_GetNamespaceFromFilename(api_name):
This isn't needed anymore since the API names are now always passed in as their
respective namespace names.

[not at google - send to devlin, 2014-07-02 22:14:34]

nice.

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:139: return
self._LookupNodeAvailability(self._GetParentPath())
could you make a bunch of these @classmethod-s to avoid the temptation of using
self._lookup_path by accident, if possible?

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:297: 'availability':
None if no_gen else self._GetAvailabilityTemplate()
why do you need to do this no_gen stuff?

in any case, passing no_gen around everywhere is pretty noisy. could you
incorporate that state into |current_node| somehow? scope the "no gen" to those
Descend calls?

[ahernandez, 2014-07-02 22:25:48]

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:139: return
self._LookupNodeAvailability(self._GetParentPath())
On 2014/07/02 22:14:34, kalman wrote:
> could you make a bunch of these @classmethod-s to avoid the temptation of
using
> self._lookup_path by accident, if possible?

I don't understand what you mean.

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:297: 'availability':
None if no_gen else self._GetAvailabilityTemplate()
On 2014/07/02 22:14:34, kalman wrote:
> why do you need to do this no_gen stuff?
> 
> in any case, passing no_gen around everywhere is pretty noisy. could you
> incorporate that state into |current_node| somehow? scope the "no gen" to
those
> Descend calls?

Certain nodes shouldn't have availability generated; no_gen is the equivalent of
blank paths in Evan's original patch. Scoping the no_gen in Descend should work.

[ahernandez, 2014-07-02 22:27:22]

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:139: return
self._LookupNodeAvailability(self._GetParentPath())
On 2014/07/02 22:25:47, ahernandez.miralles wrote:
> On 2014/07/02 22:14:34, kalman wrote:
> > could you make a bunch of these @classmethod-s to avoid the temptation of
> using
> > self._lookup_path by accident, if possible?
> 
> I don't understand what you mean.

Actually, my confusion has ended.

[ahernandez, 2014-07-03 02:04:15]

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:293: 'properties':
self._GenerateProperties(type_.properties, no_gen=no_gen),
I think eliminating the no_gen passing will make the code more awkward in
another way. On the line this comment is on, if we don't want availability data
generated for properties, we still want to generate it for functions, but if
no_gen is set to true while in the scope then the availability data won't be
generated. A possible solution is to say something like 'with
self._current_node.Descend('', no_gen=False)' where we need availability data
generated, but it seems like having to do that in certain places will be just as
noisy. WDYT?

[not at google - send to devlin, 2014-07-07 15:10:31]

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:293: 'properties':
self._GenerateProperties(type_.properties, no_gen=no_gen),
On 2014/07/03 02:04:14, ahernandez (OOO til July 6) wrote:
> I think eliminating the no_gen passing will make the code more awkward in
> another way. On the line this comment is on, if we don't want availability
data
> generated for properties, we still want to generate it for functions, but if
> no_gen is set to true while in the scope then the availability data won't be
> generated. A possible solution is to say something like 'with
> self._current_node.Descend('', no_gen=False)' where we need availability data
> generated, but it seems like having to do that in certain places will be just
as
> noisy. WDYT?

What about if no_gen took a tuple of properties to disable generation for
(no_gen=('properties')), or even just a string? Is that also too simplistic?

[ahernandez, 2014-07-07 20:04:56]

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/10001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:293: 'properties':
self._GenerateProperties(type_.properties, no_gen=no_gen),
On 2014/07/07 15:10:31, kalman wrote:
> On 2014/07/03 02:04:14, ahernandez (OOO til July 6) wrote:
> > I think eliminating the no_gen passing will make the code more awkward in
> > another way. On the line this comment is on, if we don't want availability
> data
> > generated for properties, we still want to generate it for functions, but if
> > no_gen is set to true while in the scope then the availability data won't be
> > generated. A possible solution is to say something like 'with
> > self._current_node.Descend('', no_gen=False)' where we need availability
data
> > generated, but it seems like having to do that in certain places will be
just
> as
> > noisy. WDYT?
> 
> What about if no_gen took a tuple of properties to disable generation for
> (no_gen=('properties')), or even just a string? Is that also too simplistic?

Doing that cleans up a good portion of the code, but there are still a few weird
spots where I have to call Descend() without specifying a path to descend to,
just so that a node will not have availability generated for it.

[ahernandez, 2014-07-07 21:37:14]

I've added in the changes to Descend(), PTAL.

[not at google - send to devlin, 2014-07-07 23:00:07]

looks much better, thanks.

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:136: def
_LookupParentNodeAvailability(self):
i don't see this called from anywhere?

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:169: assert 'callback'
in lookup_path, str(self)
I think if you define __repr__ rather than (or in addition to) __str__ then you
can just use |self| here not |str(self)|

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:193: may return
'parameters' as a category.
it doesn't look like this function can ever return 'parameters'

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:197: if
self._lookup_path[-1] == 'callback':
maybe add a quick comment why this is the case

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:202: return
'properties'
again this check is quite specific (especially the 'Type' check - is there a
more principled way to write that?)

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:212:
logging.warning('No availability found for: %s' % str(self))
I don't think you need the extra "str"-ing, right?

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:226: def Descend(self,
*path, **kwargs):
annoying that you can't just specify 'ignore' directly. but a neater way to
implement this I think would be to go, on line 228,

ignore = kwargs.get('ignore')

and also please comment what "ignore" is in the docstring.

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:233: self2._ignore =
kwargs['ignore']
you shouldn't need to actually use self2 here at all, just store "ignore" as
part of the outer function scope.

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:240:
self._lookup_path.extend(path)
if you're checking |path| below you might as well here too, for consistency.
doesn't make a whole lot of difference though of course.

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:381: for f in
event.filters],
this can go back to 1 line?

https://codereview.chromium.org/368973002/diff/30001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:508: with
self._current_node.Descend(*[], ignore=('types', 'properties')):
I don't think the *[] is necessary.

[ahernandez, 2014-07-07 23:51:12]

PTAL.

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
I don't think there's a better way to check for this, but I may have overlooked
a pattern.

[not at google - send to devlin, 2014-07-08 00:02:49]

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
On 2014/07/07 23:51:12, ahernandez wrote:
> I don't think there's a better way to check for this, but I may have
overlooked
> a pattern.

I see. Yeah I see this 'Type' thing breaking down, all it takes is for some
random name to end with a 'Type' which probably already happens at least with
types. Is there a reason why that particular case doesn't matter? what if
there's a function called setType() (which also seems totally reasonable)? is
there an extra check you can have for that?

maybe if you broke this condition up a little it would help, like,

if self._lookup_path[-2] == 'parameters':
  # Function parameters are modelled as properties.
  return 'properties'

if (self._lookup_path[-2].endswith('Type') and
    someothercheckwhichyoucanmake):
  # Comment.
  return 'properties'

if 'events' in self._lookup_path:
  # what does this mean? i guess that nested events are very
  # rare, so mention that the check does make sense.
  # but what if there's a property of some object called 'events'?
  # wouldn't that break?
  return 'properties'

p.s. I think you should store self._lookup_path[-2] as a variable somewhere.

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:249: return '%s > %s' %
(self._namespace_name, ' > '.join(self._lookup_path))
just do "return repr(self)"

[ahernandez, 2014-07-08 18:20:47]

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
On 2014/07/08 00:02:48, kalman wrote:
> On 2014/07/07 23:51:12, ahernandez wrote:
> > I don't think there's a better way to check for this, but I may have
> overlooked
> > a pattern.
> 
> I see. Yeah I see this 'Type' thing breaking down, all it takes is for some
> random name to end with a 'Type' which probably already happens at least with
> types. Is there a reason why that particular case doesn't matter? what if
> there's a function called setType() (which also seems totally reasonable)? is
> there an extra check you can have for that?
> 
> maybe if you broke this condition up a little it would help, like,
> 
> if self._lookup_path[-2] == 'parameters':
>   # Function parameters are modelled as properties.
>   return 'properties'
> 
> if (self._lookup_path[-2].endswith('Type') and
>     someothercheckwhichyoucanmake):
>   # Comment.
>   return 'properties'
> 
> if 'events' in self._lookup_path:
>   # what does this mean? i guess that nested events are very
>   # rare, so mention that the check does make sense.
>   # but what if there's a property of some object called 'events'?
>   # wouldn't that break?
>   return 'properties'
> 
> p.s. I think you should store self._lookup_path[-2] as a variable somewhere.

I had similar concerns about only checking for ending with 'Type'. Even if there
was a function called setType(), if we are looking up the availability for that,
the lookup path would have 'functions' right before it, and would be caught by
an earlier check. What this check catches are the cases where we have a lookup
path like 'namespace > types > myType > myTypeType', which happens quite a bit.
I can modify the check to say

if (self._lookup_path[-1].endswith('Type') and
    self._lookup_path[-1][:-len('Type')] == self._lookup_path[-2]):
  # Comment.
  return 'properties'

Regarding the if 'events' in self._lookup_path check, it's a hack to catch a
couple edge cases that don't have any sort of obvious pattern. They just
happened to have 'events' in the path...

[not at google - send to devlin, 2014-07-08 19:04:27]

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
On 2014/07/08 18:20:47, ahernandez wrote:
> On 2014/07/08 00:02:48, kalman wrote:
> > On 2014/07/07 23:51:12, ahernandez wrote:
> > > I don't think there's a better way to check for this, but I may have
> > overlooked
> > > a pattern.
> > 
> > I see. Yeah I see this 'Type' thing breaking down, all it takes is for some
> > random name to end with a 'Type' which probably already happens at least
with
> > types. Is there a reason why that particular case doesn't matter? what if
> > there's a function called setType() (which also seems totally reasonable)?
is
> > there an extra check you can have for that?
> > 
> > maybe if you broke this condition up a little it would help, like,
> > 
> > if self._lookup_path[-2] == 'parameters':
> >   # Function parameters are modelled as properties.
> >   return 'properties'
> > 
> > if (self._lookup_path[-2].endswith('Type') and
> >     someothercheckwhichyoucanmake):
> >   # Comment.
> >   return 'properties'
> > 
> > if 'events' in self._lookup_path:
> >   # what does this mean? i guess that nested events are very
> >   # rare, so mention that the check does make sense.
> >   # but what if there's a property of some object called 'events'?
> >   # wouldn't that break?
> >   return 'properties'
> > 
> > p.s. I think you should store self._lookup_path[-2] as a variable somewhere.
> 
> I had similar concerns about only checking for ending with 'Type'. Even if
there
> was a function called setType(), if we are looking up the availability for
that,
> the lookup path would have 'functions' right before it, and would be caught by
> an earlier check. What this check catches are the cases where we have a lookup
> path like 'namespace > types > myType > myTypeType', which happens quite a
bit.
> I can modify the check to say
> 
> if (self._lookup_path[-1].endswith('Type') and
>     self._lookup_path[-1][:-len('Type')] == self._lookup_path[-2]):
>   # Comment.
>   return 'properties'

that sounds good.

> 
> Regarding the if 'events' in self._lookup_path check, it's a hack to catch a
> couple edge cases that don't have any sort of obvious pattern. They just
> happened to have 'events' in the path...

which edge cases?

[ahernandez, 2014-07-08 19:20:14]

I just realized that some of the Descend() call logic broke at some point and
now availability data is not being found when it was before. I'll ping the issue
when it's ready (and actually working).

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
On 2014/07/08 19:04:27, kalman wrote:
> On 2014/07/08 18:20:47, ahernandez wrote:
> > On 2014/07/08 00:02:48, kalman wrote:
> > > On 2014/07/07 23:51:12, ahernandez wrote:
> > > > I don't think there's a better way to check for this, but I may have
> > > overlooked
> > > > a pattern.
> > > 
> > > I see. Yeah I see this 'Type' thing breaking down, all it takes is for
some
> > > random name to end with a 'Type' which probably already happens at least
> with
> > > types. Is there a reason why that particular case doesn't matter? what if
> > > there's a function called setType() (which also seems totally reasonable)?
> is
> > > there an extra check you can have for that?
> > > 
> > > maybe if you broke this condition up a little it would help, like,
> > > 
> > > if self._lookup_path[-2] == 'parameters':
> > >   # Function parameters are modelled as properties.
> > >   return 'properties'
> > > 
> > > if (self._lookup_path[-2].endswith('Type') and
> > >     someothercheckwhichyoucanmake):
> > >   # Comment.
> > >   return 'properties'
> > > 
> > > if 'events' in self._lookup_path:
> > >   # what does this mean? i guess that nested events are very
> > >   # rare, so mention that the check does make sense.
> > >   # but what if there's a property of some object called 'events'?
> > >   # wouldn't that break?
> > >   return 'properties'
> > > 
> > > p.s. I think you should store self._lookup_path[-2] as a variable
somewhere.
> > 
> > I had similar concerns about only checking for ending with 'Type'. Even if
> there
> > was a function called setType(), if we are looking up the availability for
> that,
> > the lookup path would have 'functions' right before it, and would be caught
by
> > an earlier check. What this check catches are the cases where we have a
lookup
> > path like 'namespace > types > myType > myTypeType', which happens quite a
> bit.
> > I can modify the check to say
> > 
> > if (self._lookup_path[-1].endswith('Type') and
> >     self._lookup_path[-1][:-len('Type')] == self._lookup_path[-2]):
> >   # Comment.
> >   return 'properties'
> 
> that sounds good.
> 
> > 
> > Regarding the if 'events' in self._lookup_path check, it's a hack to catch a
> > couple edge cases that don't have any sort of obvious pattern. They just
> > happened to have 'events' in the path...
> 
> which edge cases?

One example is 'webviewTag > events > consolemessage > level'. I don't see how
that can be classified by just looking at the path.

[not at google - send to devlin, 2014-07-08 19:27:17]

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
> One example is 'webviewTag > events > consolemessage > level'. I don't see how
> that can be classified by just looking at the path.

ah I see. yeah that does sound event specific; I presume that if
'consolemessage' were a function then it would look like

webviewTag > functions > consolemessage > *parameters* > level

pity that we don't generate that extra "parameters" there, but maybe that would
create its own problems. the model we have is fine.

what if you checked explicitly for 'events' at lookup_path[whatever] and added a
comment to that effect?

[ahernandez, 2014-07-08 20:19:27]

It looks like if the docs are not inlined, then there isn't enough information
available to generate availability data for certain nodes. How should this be
dealt with?

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/70001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1].endswith('Type') or
On 2014/07/08 19:27:17, kalman wrote:
> > One example is 'webviewTag > events > consolemessage > level'. I don't see
how
> > that can be classified by just looking at the path.
> 
> ah I see. yeah that does sound event specific; I presume that if
> 'consolemessage' were a function then it would look like
> 
> webviewTag > functions > consolemessage > *parameters* > level
> 
> pity that we don't generate that extra "parameters" there, but maybe that
would
> create its own problems. the model we have is fine.
> 
> what if you checked explicitly for 'events' at lookup_path[whatever] and added
a
> comment to that effect?

Sounds fine.

[not at google - send to devlin, 2014-07-08 20:54:52]

On 2014/07/08 20:19:27, ahernandez wrote:
> It looks like if the docs are not inlined, then there isn't enough information
> available to generate availability data for certain nodes. How should this be
> dealt with?

which nodes?

[ahernandez, 2014-07-08 21:01:11]

On 2014/07/08 20:54:52, kalman wrote:
> On 2014/07/08 20:19:27, ahernandez wrote:
> > It looks like if the docs are not inlined, then there isn't enough
information
> > available to generate availability data for certain nodes. How should this
be
> > dealt with?
> 
> which nodes?

There are a lot, but one example is fileBrowserPrivate > types > ProfileInfo >
properties > profileImage > properties > scale1xUrl. It seems like it is always
related to properties of properties. I looked at the schema graphs with inlining
and without, and before inlining a lot of those more deeply nested property
structures are not there.

[not at google - send to devlin, 2014-07-08 21:13:36]

On 2014/07/08 21:01:11, ahernandez wrote:
> On 2014/07/08 20:54:52, kalman wrote:
> > On 2014/07/08 20:19:27, ahernandez wrote:
> > > It looks like if the docs are not inlined, then there isn't enough
> information
> > > available to generate availability data for certain nodes. How should this
> be
> > > dealt with?
> > 
> > which nodes?
> 
> There are a lot, but one example is fileBrowserPrivate > types > ProfileInfo >
> properties > profileImage > properties > scale1xUrl. It seems like it is
always
> related to properties of properties. I looked at the schema graphs with
inlining
> and without, and before inlining a lot of those more deeply nested property
> structures are not there.

ok it sounds like that's not too much of a worry right now

[ahernandez, 2014-07-08 21:39:39]

I reverted the doc inlining back to the way it used to be and addressed the
other comments. PTAL

[not at google - send to devlin, 2014-07-08 21:47:57]

On 2014/07/08 21:39:39, ahernandez wrote:
> I reverted the doc inlining back to the way it used to be and addressed the
> other comments. PTAL

oh, I mean that you don't have to change anything for this patch? the existing
behaviour seems better?

[ahernandez, 2014-07-08 21:56:25]

On 2014/07/08 21:47:57, kalman wrote:
> On 2014/07/08 21:39:39, ahernandez wrote:
> > I reverted the doc inlining back to the way it used to be and addressed the
> > other comments. PTAL
> 
> oh, I mean that you don't have to change anything for this patch? the existing
> behaviour seems better?

The way the patch was before, availability data wasn't being generated for those
deeply nested structures. I changed it so that it is being generated. Did you
mean that it was fine if those structures weren't being generated right now? It
produces a lot of warnings.

[not at google - send to devlin, 2014-07-08 22:01:24]

On 2014/07/08 21:56:25, ahernandez wrote:
> On 2014/07/08 21:47:57, kalman wrote:
> > On 2014/07/08 21:39:39, ahernandez wrote:
> > > I reverted the doc inlining back to the way it used to be and addressed
the
> > > other comments. PTAL
> > 
> > oh, I mean that you don't have to change anything for this patch? the
existing
> > behaviour seems better?
> 
> The way the patch was before, availability data wasn't being generated for
those
> deeply nested structures. I changed it so that it is being generated. Did you
> mean that it was fine if those structures weren't being generated right now?
It
> produces a lot of warnings.

I think not generating for deeply nested structures is less misleading than
generating false information when things have been inlined.

[ahernandez, 2014-07-08 22:19:33]

Ok, here are the changes with doc inlining behavior preserved. PTAL

[not at google - send to devlin, 2014-07-08 22:22:42]

lgtm

https://codereview.chromium.org/368973002/diff/150001/chrome/common/extension...
File chrome/common/extensions/docs/server2/api_data_source.py (right):

https://codereview.chromium.org/368973002/diff/150001/chrome/common/extension...
chrome/common/extensions/docs/server2/api_data_source.py:201:
self._lookup_path[-1][:-len('ReturnType')] == self._lookup_path[-2]):
use parentheses when mixing "and" and "or" in the same statement.

[ahernandez, 2014-07-08 22:38:11]

The CQ bit was checked by ahernandez.miralles@gmail.com

[commit-bot: I haz the power, 2014-07-08 22:43:48]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/ahernandez.miralles@gmail.com/36897300...

[commit-bot: I haz the power, 2014-07-09 05:24:39]

Change committed as 281952