Docserver: Show callback parameters in all callback references.

Docserver: Show callback parameters in all callback references.

BUG=395430
NOTRY=True

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

[ahernandez, 2014-07-21 21:52:07]

I believe this solves the issue with callback documentation not showing up.
PTAL.

[ahernandez, 2014-07-21 21:52:07]

I believe this solves the issue with callback documentation not showing up.
PTAL.

[not at google - send to devlin, 2014-07-21 22:49:47]

lgtm with some follow-up investigation.

https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
File chrome/common/extensions/docs/templates/private/callback.html (right):

https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
chrome/common/extensions/docs/templates/private/callback.html:22:
parentCallback:callback
Thanks for doing this. I'm trying to figure out why this is called
"parentCallback" and not just "callback", but, failing. I'm also seeing a number
of other uses of parameter_full without specifying a parentCallback:
- articles/settings_override.html and articles/ui_override.html
- private/api_property.html
- private/event.html
- private/property.html (twice)
- private/type_item.html (from all the various places that "item_partial" is
passed in without a parentCallback)

Do any of those mean anything to you?

[ahernandez, 2014-07-21 23:27:39]

On 2014/07/21 22:49:47, kalman wrote:
> lgtm with some follow-up investigation.
> 
>
https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
> File chrome/common/extensions/docs/templates/private/callback.html (right):
> 
>
https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
> chrome/common/extensions/docs/templates/private/callback.html:22:
> parentCallback:callback
> Thanks for doing this. I'm trying to figure out why this is called
> "parentCallback" and not just "callback", but, failing. I'm also seeing a
number
> of other uses of parameter_full without specifying a parentCallback:
> - articles/settings_override.html and articles/ui_override.html
> - private/api_property.html
> - private/event.html
> - private/property.html (twice)
> - private/type_item.html (from all the various places that "item_partial" is
> passed in without a parentCallback)
> 
> Do any of those mean anything to you?

I can't figure out why it's called parentCallback either, the name doesn't make
any sense to me.

I'm familiar with a couple of those files. I can't tell why parentCallback isn't
passed in in certain areas, but it looks like it shouldn't matter if it is
passed in. Do you want me to try passing in parentCallback in those files and
upload another patchset?

[not at google - send to devlin, 2014-07-21 23:36:10]

On 2014/07/21 23:27:39, ahernandez wrote:
> On 2014/07/21 22:49:47, kalman wrote:
> > lgtm with some follow-up investigation.
> > 
> >
>
https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
> > File chrome/common/extensions/docs/templates/private/callback.html (right):
> > 
> >
>
https://codereview.chromium.org/406933003/diff/1/chrome/common/extensions/doc...
> > chrome/common/extensions/docs/templates/private/callback.html:22:
> > parentCallback:callback
> > Thanks for doing this. I'm trying to figure out why this is called
> > "parentCallback" and not just "callback", but, failing. I'm also seeing a
> number
> > of other uses of parameter_full without specifying a parentCallback:
> > - articles/settings_override.html and articles/ui_override.html
> > - private/api_property.html
> > - private/event.html
> > - private/property.html (twice)
> > - private/type_item.html (from all the various places that "item_partial" is
> > passed in without a parentCallback)
> > 
> > Do any of those mean anything to you?
> 
> I can't figure out why it's called parentCallback either, the name doesn't
make
> any sense to me.
> 
> I'm familiar with a couple of those files. I can't tell why parentCallback
isn't
> passed in in certain areas, but it looks like it shouldn't matter if it is
> passed in. Do you want me to try passing in parentCallback in those files and
> upload another patchset?

What would it look like? I remember occasionally seeing places where we miss the
description of some callbacks.

might we well add them in if you're sure they actually make sense, but yeah,
given I don't know what "parentCallback" means it's hard to say.

if you do figure out what it's for and can give it a better name, or make it
unnecessary... that'd be a big improvement.

[ahernandez, 2014-07-22 19:34:04]

I removed the parentCallback property entirely, it didn't seem necessary. PTAL.

[not at google - send to devlin, 2014-07-23 03:25:04]

sorry this took so long.

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

https://codereview.chromium.org/406933003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/api_data_source.py:517: def
_GenerateCallbackProperty(self, callback, callback_dict):
kinda weird to be passing in |callback_dict| when you can determine it from
|callback| but it does save doing the same work twice, so worth it in that
respect.

I think this is all because once upon a time a callback was just modelled as a
function. Like, there was no "GenerateCallbackProperty" there was just
"GenerateFunction". However that caused some sort of issue so it was split up?

but that's the funny part - GenerateFunction *does* have a 'callback' property
already, and it actually means a callback to that function. That's not what
it'll mean here, and I'm not sure what that will do.

would it work to have 2 properties, "isCallback" and "asFunction"? is that
enough to get by?

https://codereview.chromium.org/406933003/diff/20001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/cron.yaml (right):

https://codereview.chromium.org/406933003/diff/20001/chrome/common/extensions...
chrome/common/extensions/docs/server2/cron.yaml:5: target: 3-33-0
make sure you rebase :)

[ahernandez, 2014-07-23 17:38:27]

How's this?

[not at google - send to devlin, 2014-07-23 18:12:03]

lgtm

https://codereview.chromium.org/406933003/diff/40001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/private/callback.html (right):

https://codereview.chromium.org/406933003/diff/40001/chrome/common/extensions...
chrome/common/extensions/docs/templates/private/callback.html:22:
hideParamTOC:true /}}
this is just a (unnecessary) formatting change right? you can revert it?

[ahernandez, 2014-07-23 22:03:25]

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

[commit-bot: I haz the power, 2014-07-23 22:07:09]

CQ is trying da patch. Follow status at

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

[commit-bot: I haz the power, 2014-07-23 22:10:19]

Change committed as 285056