Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
216473005:
Settings Override API: update documentation. (Closed)
|
Created:
6 years, 9 months ago by vasilii Modified:
6 years, 8 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionSettings Override API: update documentation.
The documentation describes usage of the "Install Param" for external extensions.
BUG=267510
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263125
Patch Set 1 #
Total comments: 2
Patch Set 2 : ref:i18n #
Total comments: 8
Patch Set 3 : @mkearney comments #
Total comments: 8
Patch Set 4 : @battre comments #
Messages
Total messages: 20 (0 generated)
Hi, please review the documentation.
Meggin is our technical writer for extensions.
yes you'd better get meggin to look at this one. https://codereview.chromium.org/216473005/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/templates/articles/settings_override.html:55: <a href="http://developer.chrome.com/extensions/i18n">chrome.i18n API</a>. you should be able to use $(ref:i18n chrome.i18n API) instead of the full <a...> syntax.
https://codereview.chromium.org/216473005/diff/1/chrome/common/extensions/doc... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/1/chrome/common/extensions/doc... chrome/common/extensions/docs/templates/articles/settings_override.html:55: <a href="http://developer.chrome.com/extensions/i18n">chrome.i18n API</a>. On 2014/03/28 14:36:25, kalman wrote: > you should be able to use $(ref:i18n chrome.i18n API) instead of the full <a...> > syntax. Done.
https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... File chrome/common/extensions/api/manifest_types.json (right): https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/api/manifest_types.json:154: "description": "An array of length one containing a URL to be used as the startup page.", Is it normal to use an array when there's only one item? https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:55: $(ref:i18n chrome.i18n API). Simplify: The <code>search_provider</code>... properties can be localized using the chrome.i18n API. https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:59: all URLs values of <code>search_provider</code>, <code>homepage</code> and How about after the comma: "the <code>search_provider</code>... and <code>startup_pages</code> URL values can be parametrized... https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are just removed. Note that "__PARAM__" must be beyond the You switch between singular and plural here. Stick to one: If "install_parameter" is absent, all "_PARAM_" are removed. Note that "_PARAM_"... they cannot be part of the hostname.
https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... File chrome/common/extensions/api/manifest_types.json (right): https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/api/manifest_types.json:154: "description": "An array of length one containing a URL to be used as the startup page.", On 2014/03/28 23:02:20, mkearney1 wrote: > Is it normal to use an array when there's only one item? Yes, because we can allow more than 1 URL in the future. At the same time the JSON schema stays intact. https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:55: $(ref:i18n chrome.i18n API). On 2014/03/28 23:02:20, mkearney1 wrote: > Simplify: The <code>search_provider</code>... properties can be localized using > the chrome.i18n API. Done. https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:59: all URLs values of <code>search_provider</code>, <code>homepage</code> and On 2014/03/28 23:02:20, mkearney1 wrote: > How about after the comma: "the <code>search_provider</code>... and > <code>startup_pages</code> URL values can be parametrized... Done. https://codereview.chromium.org/216473005/diff/20001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are just removed. Note that "__PARAM__" must be beyond the On 2014/03/28 23:02:20, mkearney1 wrote: > You switch between singular and plural here. Stick to one: > > If "install_parameter" is absent, all "_PARAM_" are removed. Note that > "_PARAM_"... they cannot be part of the hostname. Done.
ping
Some proposals from my side. https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:62: instructions <a href=http://developer.chrome.com/extensions/external_extensions#registry>here</a>). "" around href parameter? https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:63: The value name is "install_parameter", the value data is an arbitrary string: s/value name/entry name/ s/value data/entry data/ https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are removed. Note that "__PARAM__" must be beyond the absent, occurrences of "__PARAM__" https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are removed. Note that "__PARAM__" must be beyond the Note that "__PARAM__" cannot be part of the hostname. It needs to occur after the first '/' in the URL.
https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/settings_override.html (right): https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:62: instructions <a href=http://developer.chrome.com/extensions/external_extensions#registry>here</a>). On 2014/04/01 14:15:44, battre wrote: > "" around href parameter? Done. https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:63: The value name is "install_parameter", the value data is an arbitrary string: On 2014/04/01 14:15:44, battre wrote: > s/value name/entry name/ > s/value data/entry data/ This is official Microsoft terminology. Data entries are called values http://msdn.microsoft.com/en-us/library/windows/desktop/ms724946(v=vs.85).aspx https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are removed. Note that "__PARAM__" must be beyond the On 2014/04/01 14:15:44, battre wrote: > absent, occurrences of "__PARAM__" Done. https://codereview.chromium.org/216473005/diff/40001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/settings_override.html:70: absent, "__PARAM__" are removed. Note that "__PARAM__" must be beyond the On 2014/04/01 14:15:44, battre wrote: > Note that "__PARAM__" cannot be part of the hostname. It needs to occur after > the first '/' in the URL. Done.
lgtm
@mkearney, @kalman, do you have more comments?
lgtm if battre is happy, meggin do you have any more comments?
The CQ bit was checked by vasilii@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vasilii@chromium.org/216473005/50001
The CQ bit was unchecked by commit-bot@chromium.org
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...
lgtm
The CQ bit was checked by vasilii@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vasilii@chromium.org/216473005/50001
Message was sent while issue was closed.
Change committed as 263125 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
