Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2896503002:
[Extensions Bindings] Add some documentation (Closed)
Description[Extensions Bindings] Add some documentation
Add a bindings.md file containing some documentation about the bindings
system.
BUG=653596
Review-Url: https://codereview.chromium.org/2896503002
Cr-Commit-Position: refs/heads/master@{#474098}
Committed: https://chromium.googlesource.com/chromium/src/+/c9da424575bfae181e86289cefbbf589ab31e1c8
Patch Set 1 #
Total comments: 14
Patch Set 2 : jbroman's #Patch Set 3 : linkify #
Total comments: 6
Patch Set 4 : nits #Messages
Total messages: 19 (9 generated)
rdevlin.cronin@chromium.org changed reviewers: + jbroman@chromium.org, lazyboy@chromium.org
Heya folks, mind taking a look? Let me know what gaps there are here. (I'm not trying to document everything in one fell swoop, but if there's gaping holes, it'd be good to start to fill them!) pro-tip: patch the CL in and use `tools/md_browser/md_browser.py extensions/renderer/bindings.md` to see the markdown.
On 2017/05/18 at 21:59:14, rdevlin.cronin wrote: > Heya folks, mind taking a look? Let me know what gaps there are here. (I'm not trying to document everything in one fell swoop, but if there's gaping holes, it'd be good to start to fill them!) > > pro-tip: patch the CL in and use `tools/md_browser/md_browser.py extensions/renderer/bindings.md` to see the markdown. Reciprocal pro-tip: if you upload to Gerrit, you can view the markdown as rendered by gitiles from the code review itself (by clicking on the file, then "gitiles"). See https://chromium-review.googlesource.com/c/481339/ as an example. :)
Yay documentation. Consider linking to this from extensions/README.md? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... File extensions/renderer/bindings.md (right): https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:13: Bindings are initialized by creating an ObjectTemplate from an API specification nit: Describe where I could find these specifications, and/or documentation about them? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:61: Properties Feature restrictions are based on a specific v8::Context. Different nit: Delete "Properties", or clarify if this was not a typo? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:66: the feature documentation. nit: link to "the feature documentation"? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:81: is included, a pending request is added. nit: What does it mean for "a pending request to be added"? Added to what? Maybe something along the lines of "If a callback is provided in the arguments, it is stored until the response is received."? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:91: typical behavior. OK, so these hooks exist. Where would I look to attempt to register such a hook? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:109: Allows an AIP implementation to add a callback that should be called with the nit: s/AIP/API/? https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:196: Signature matching differs significantly between open web functions and "open web functions" == WebIDL?
https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... File extensions/renderer/bindings.md (right): https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:13: Bindings are initialized by creating an ObjectTemplate from an API specification On 2017/05/23 18:58:25, jbroman wrote: > nit: Describe where I could find these specifications, and/or documentation > about them? Done. (We need to add documentation about those, as well) https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:61: Properties Feature restrictions are based on a specific v8::Context. Different On 2017/05/23 18:58:25, jbroman wrote: > nit: Delete "Properties", or clarify if this was not a typo? Whoops! Format fail. Fixed. https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:66: the feature documentation. On 2017/05/23 18:58:25, jbroman wrote: > nit: link to "the feature documentation"? Done. https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:81: is included, a pending request is added. On 2017/05/23 18:58:25, jbroman wrote: > nit: What does it mean for "a pending request to be added"? Added to what? Maybe > something along the lines of "If a callback is provided in the arguments, it is > stored until the response is received."? Done. https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:91: typical behavior. On 2017/05/23 18:58:25, jbroman wrote: > OK, so these hooks exist. Where would I look to attempt to register such a hook? Done. https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:109: Allows an AIP implementation to add a callback that should be called with the On 2017/05/23 18:58:25, jbroman wrote: > nit: s/AIP/API/? Done. https://codereview.chromium.org/2896503002/diff/1/extensions/renderer/binding... extensions/renderer/bindings.md:196: Signature matching differs significantly between open web functions and On 2017/05/23 18:58:25, jbroman wrote: > "open web functions" == WebIDL? Done.
lgtm https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... File extensions/renderer/bindings.md (right): https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:66: the [feature documentation](https://chromium.googlesource.com/chromium/src/+/master/chrome/common/extensions/api/_features.md). nit: you can use relative and repository-relative links in gitiles md, which will automatically link to the same revision being viewed: [feature documentation](../../chrome/common/extensions/api/_features.md) or [feature documentation](/chrome/extensions/api/_features.md)
On 2017/05/23 at 20:46:09, jbroman wrote: > lgtm > > https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... > File extensions/renderer/bindings.md (right): > > https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... > extensions/renderer/bindings.md:66: the [feature documentation](https://chromium.googlesource.com/chromium/src/+/master/chrome/common/extensions/api/_features.md). > nit: you can use relative and repository-relative links in gitiles md, which will automatically link to the same revision being viewed: > > [feature documentation](../../chrome/common/extensions/api/_features.md) > or > [feature documentation](/chrome/extensions/api/_features.md) err, forgot the /common/ component of the path in the latter example: [feature documentation](/chrome/common/extensions/api/_features.md)
lgtm https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... File extensions/renderer/bindings.md (right): https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:5: ## What Is It nit: What Is It? or What It Is https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:150: event happens; rather, the browser takes immediately takes the specified action. the browser immediately takes..
The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... File extensions/renderer/bindings.md (right): https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:5: ## What Is It On 2017/05/23 22:20:19, lazyboy wrote: > nit: What Is It? or What It Is Done. https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:66: the [feature documentation](https://chromium.googlesource.com/chromium/src/+/master/chrome/common/extensions/api/_features.md). On 2017/05/23 20:46:09, jbroman wrote: > nit: you can use relative and repository-relative links in gitiles md, which > will automatically link to the same revision being viewed: > > [feature documentation](../../chrome/common/extensions/api/_features.md) > or > [feature documentation](/chrome/extensions/api/_features.md) Done. https://codereview.chromium.org/2896503002/diff/40001/extensions/renderer/bin... extensions/renderer/bindings.md:150: event happens; rather, the browser takes immediately takes the specified action. On 2017/05/23 22:20:19, lazyboy wrote: > the browser immediately takes.. Done.
The CQ bit was checked by rdevlin.cronin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lazyboy@chromium.org, jbroman@chromium.org Link to the patchset: https://codereview.chromium.org/2896503002/#ps60001 (title: "nits")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495580991508100,
"parent_rev": "7ade5b542808909fcf16f86935d6a22e22f35ffd", "commit_rev":
"c9da424575bfae181e86289cefbbf589ab31e1c8"}
Message was sent while issue was closed.
Description was changed from ========== [Extensions Bindings] Add some documentation Add a bindings.md file containing some documentation about the bindings system. BUG=653596 ========== to ========== [Extensions Bindings] Add some documentation Add a bindings.md file containing some documentation about the bindings system. BUG=653596 Review-Url: https://codereview.chromium.org/2896503002 Cr-Commit-Position: refs/heads/master@{#474098} Committed: https://chromium.googlesource.com/chromium/src/+/c9da424575bfae181e86289cefbb... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/c9da424575bfae181e86289cefbb... |
