Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
145443002:
GCM for Chrome documentation (Closed)
|
Created:
6 years, 11 months ago by jianli Modified:
6 years, 9 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, fgorski, rpop Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionGCM for Chrome documentation
BUG=284553
R=kalman@chromium.org, mkearney@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255671
Patch Set 1 #Patch Set 2 : Patch #
Total comments: 73
Patch Set 3 : Address comments #Patch Set 4 : Some changes #Patch Set 5 : Add missing full stop #
Total comments: 1
Patch Set 6 : #Patch Set 7 : #
Messages
Total messages: 24 (0 generated)
Meggin, could you please take a look? Thanks.
I'm looking at this now-- lots of review comments so stay tuned. On Tue, Feb 4, 2014 at 12:58 PM, <jianli@chromium.org> wrote: > Reviewers: mkearney1, > > Message: > Meggin, could you please take a look? Thanks. > > Description: > GCM for Chrome documentation > > BUG=284553 > > Please review this at https://codereview.chromium.org/145443002/ > > SVN Base: svn://svn.chromium.org/chrome/trunk/src > > Affected files (+304, -8 lines): > A chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html > M chrome/common/extensions/docs/templates/json/apps_sidenav.json > M chrome/common/extensions/docs/templates/json/chrome_sidenav.json > M chrome/common/extensions/docs/templates/json/intro_tables.json > A chrome/common/extensions/docs/templates/public/apps/ > cloudMessagingV2.html > > > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I've marked a couple of items below as IMPORTANT. Give me a shout if you need clarification. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html (right): https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:4: <h2 id="t1">Overview</h2> This content doesn't need to be in an overview (h2). Just have it follow the h1. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:11: upstream messages to the server.</p> Is there a working sample for new GCM? There definitely should be! https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:12: You don't mention anywhere that this version of GCM is dev channel only. Is that because it won't be for long? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:15: the <a href="http://developer.android.com/google/gcm/index.html">GCM Service</a>. To get Why are we pointing to android docs instead of gcm_server.html? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:16: help with GCM or to give us feedback, see <a href="#feedback">Feedback</a>.</p> Why have we removed the Feedback section? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:17: Make headers consistent! Suggestion-- h3 should be h2 and read: Enable GCM https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:23: <li>Upload your app or extension client to the Chrome Web Store.</li> Link to webstore/publish.html. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:25: <li>Your app or extension client requests a registration ID using $ref:gcm.register Link to related doc (obtaining registration ID) for more information. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:30: <h3 id="t3">How to receive a message</h3> Make headers consistent! Suggestion-- h3 should be h2 and read: Receive a message https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:44: message.</li> Link to related doc (Receiving messages). https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:47: <h3 id="t4">How to send a message</h3> Make headers consistent! Suggestion-- h3 should be h2 and read: Send a message https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:55: message ID, destination ID that identifies the server, and data that consist Link to related doc (Sending messages). https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:64: the message.</li> Link to Error reference section. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:66: Make headers consistent! Slight rewording of title: Set up project https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:69: <h3 id="t6">Creating a Google API project</h3> Make headers consistent! Slight rewording of title: Create Google API Project https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:82: <h3 id="t7">Enabling the GCM service</h3> Enable the GCM service https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:87: API</strong> toggle to ON.</li> Is it worth mentioning the terms of service here? Also, have those terms changed, and if so, the doc may need to be updated! Last update, September 2012. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:90: <h2 id="t8">Setting up Chrome app or extension</h2> Set up Chrome App or Extension https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:93: Add permission to manifest https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:99: "gcm", The , looks funny here without the ... https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:103: <h2 id="t10">Writing Chrome app or extension</h2> Write Chrome App or Extension https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:105: <h3 id="t11">Obtaining registration ID</h3> Obtain registration ID https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:145: Weird couple of sentences here. How about: Your app or extension is automatically unregistered from the GCM service when a user uninstalls it. You can't unregister using the GCM API. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:148: Receive messages https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:158: <p>Messages from the server are delivered via $ref:gcm.onMessage event. Your app or Nit-- via the instead of just via. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:166: // consists of key-value pairs. Code isn't valid. Check your blocks. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:171: <p>});</p> I think you need to remove this blockquote. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:177: <h3 id="t13">Sending messages</h3> Send messages https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:181: IMPORTANT-- the terminology in this bullet list doesn't match exactly the terminology in the sample code below and it's important that it does so the developer can map what they are reading to the sample. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:182: <ul> You should capitalize the first letters in each bullet. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:187: via chrome.storage API and restores it when the app reloads.</li> Nit-- 'restores it' should be 'restore it'. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:191: total)</li> Nit-- Line 91 list item missing a full-stop. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:263: <h2 id="t15">Publishing your app</h2> Publish your app https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:266: Store</a>.</p> Link instead to webstore/publish.html https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:294: <h2 id="feedback"></h2> Missing text! Feedback https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:297: the Google Group <a href="https://groups.google.com/forum/#!forum/gcm-for-chrome-v2-feedback">GCM for Chrome V2 There isn't a group set up for this yet!
https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html (right): https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:4: <h2 id="t1">Overview</h2> On 2014/02/04 21:23:45, mkearney1 wrote: > This content doesn't need to be in an overview (h2). Just have it follow the h1. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:11: upstream messages to the server.</p> On 2014/02/04 21:23:45, mkearney1 wrote: > Is there a working sample for new GCM? There definitely should be! We don't have sample yet. We are definitely going to publish a sample app on gitpub at later time, probably before publishing the document. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:12: On 2014/02/04 21:23:45, mkearney1 wrote: > You don't mention anywhere that this version of GCM is dev channel only. Is that > because it won't be for long? We're going to ship GCM API to stable channel for M35. This document will be published sometime after M35 branch point (3/31). Do we need to mention that GCM API is dev channel only. Once it hits stable channel, we will update the document to remove this sentence. Right? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:15: the <a href="http://developer.android.com/google/gcm/index.html">GCM Service</a>. To get On 2014/02/04 21:23:45, mkearney1 wrote: > Why are we pointing to android docs instead of gcm_server.html? Our existing documentation for GCM service at http://developer.chrome.com/apps/gcm_server.html is for pushing messaging. We're now using Android GCM service. This is the reason I point to it. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:16: help with GCM or to give us feedback, see <a href="#feedback">Feedback</a>.</p> On 2014/02/04 21:23:45, mkearney1 wrote: > Why have we removed the Feedback section? This is due to conversion error. Fixed. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:17: On 2014/02/04 21:23:45, mkearney1 wrote: > Make headers consistent! > > Suggestion-- h3 should be h2 and read: > > Enable GCM Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:23: <li>Upload your app or extension client to the Chrome Web Store.</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Link to webstore/publish.html. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:25: <li>Your app or extension client requests a registration ID using $ref:gcm.register On 2014/02/04 21:23:45, mkearney1 wrote: > Link to related doc (obtaining registration ID) for more information. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:30: <h3 id="t3">How to receive a message</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Make headers consistent! > > Suggestion-- h3 should be h2 and read: > > Receive a message If we get rid of <h2>Overview</h2> and promote "Receive a message" to h2. It seems to be get easily confused with "Receive messages" in "Write Chrome App or Extension" h2 section. Here we want to summarize what to do to receive a message. And "Receive messages" gives the details steps and sample code on how to do it? How are we going to organize these sections in better way? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:44: message.</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Link to related doc (Receiving messages). Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:47: <h3 id="t4">How to send a message</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Make headers consistent! > > Suggestion-- h3 should be h2 and read: > > Send a message Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:55: message ID, destination ID that identifies the server, and data that consist On 2014/02/04 21:23:45, mkearney1 wrote: > Link to related doc (Sending messages). Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:64: the message.</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Link to Error reference section. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:66: On 2014/02/04 21:23:45, mkearney1 wrote: > Make headers consistent! > > Slight rewording of title: > > Set up project Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:69: <h3 id="t6">Creating a Google API project</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Make headers consistent! > > Slight rewording of title: > > Create Google API Project Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:82: <h3 id="t7">Enabling the GCM service</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Enable the GCM service Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:87: API</strong> toggle to ON.</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Is it worth mentioning the terms of service here? Also, have those terms > changed, and if so, the doc may need to be updated! Last update, September 2012. I added the text to mention TOS at the beginning of this doc. I do not think we need a separate a specific TOS. Instead, we should refer to both standard TOS for API and Cloud service. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:90: <h2 id="t8">Setting up Chrome app or extension</h2> On 2014/02/04 21:23:45, mkearney1 wrote: > Set up Chrome App or Extension Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:93: On 2014/02/04 21:23:45, mkearney1 wrote: > Add permission to manifest Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:99: "gcm", On 2014/02/04 21:23:45, mkearney1 wrote: > The , looks funny here without the ... Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:103: <h2 id="t10">Writing Chrome app or extension</h2> On 2014/02/04 21:23:45, mkearney1 wrote: > Write Chrome App or Extension Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:105: <h3 id="t11">Obtaining registration ID</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Obtain registration ID Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:145: On 2014/02/04 21:23:45, mkearney1 wrote: > Weird couple of sentences here. How about: > > Your app or extension is automatically unregistered from the GCM service when a > user uninstalls it. You can't unregister using the GCM API. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:148: On 2014/02/04 21:23:45, mkearney1 wrote: > Receive messages Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:158: <p>Messages from the server are delivered via $ref:gcm.onMessage event. Your app or On 2014/02/04 21:23:45, mkearney1 wrote: > Nit-- via the instead of just via. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:166: // consists of key-value pairs. On 2014/02/04 21:23:45, mkearney1 wrote: > Code isn't valid. Check your blocks. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:171: <p>});</p> On 2014/02/04 21:23:45, mkearney1 wrote: > I think you need to remove this blockquote. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:177: <h3 id="t13">Sending messages</h3> On 2014/02/04 21:23:45, mkearney1 wrote: > Send messages Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:181: On 2014/02/04 21:23:45, mkearney1 wrote: > IMPORTANT-- the terminology in this bullet list doesn't match exactly the > terminology in the sample code below and it's important that it does so the > developer can map what they are reading to the sample. Which part does not match? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:182: <ul> On 2014/02/04 21:23:45, mkearney1 wrote: > You should capitalize the first letters in each bullet. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:187: via chrome.storage API and restores it when the app reloads.</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Nit-- 'restores it' should be 'restore it'. Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:191: total)</li> On 2014/02/04 21:23:45, mkearney1 wrote: > Nit-- Line 91 list item missing a full-stop. Not sure I understand this. What do you mean by full-stop? https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:263: <h2 id="t15">Publishing your app</h2> On 2014/02/04 21:23:45, mkearney1 wrote: > Publish your app Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:266: Store</a>.</p> On 2014/02/04 21:23:45, mkearney1 wrote: > Link instead to webstore/publish.html Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:294: <h2 id="feedback"></h2> On 2014/02/04 21:23:45, mkearney1 wrote: > Missing text! Feedback Done. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:297: the Google Group <a href="https://groups.google.com/forum/#!forum/gcm-for-chrome-v2-feedback">GCM for Chrome V2 On 2014/02/04 21:23:45, mkearney1 wrote: > There isn't a group set up for this yet! Will create one once before we official launch the API.
generally looks good. https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... File chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html (right): https://codereview.chromium.org/145443002/diff/30001/chrome/common/extensions... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:191: total)</li> On 2014/02/05 22:04:56, jianli wrote: > On 2014/02/04 21:23:45, mkearney1 wrote: > > Nit-- Line 91 list item missing a full-stop. > > Not sure I understand this. What do you mean by full-stop? It's a period ('.') at the end of the sentence. e.g. ^
lgtm... the one thing i'm unclear about in the sample code (send message) is the destination id-- what's this called?
one more comment on onMessagesDeleted https://codereview.chromium.org/145443002/diff/250001/chrome/common/extension... File chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html (right): https://codereview.chromium.org/145443002/diff/250001/chrome/common/extension... chrome/common/extensions/docs/templates/articles/cloudMessagingV2.html:288: <h3 id="t10">Maximum number of messages stored in GCM</h3> we could add an <a href="#on_messages_deleted"> somewhere here, so that we can refer to is from the gcm.json. We don't duplicate the information then, and we can address this comment from Costin in one place: onMessageDeleted: probably some extra info ( from android page: "GCM will store up to 100 non-collapsible messages. After that, all messages are discarded from GCM, and a new message is created that tells the client how far behind it is."
On 2014/02/13 00:38:22, mkearney1 wrote: > lgtm... > > the one thing i'm unclear about in the sample code (send message) is the > destination id-- what's this called? Could you please take one more look? I have made some changes to the document. Also, we want to keep both the existing push messaging document and new GCM document. How do we want to name these two differently? Currently, I rename "Google Cloud Messaging" used in push messaging document to "Google Cloud Messaging for Chrome", while use "Google Cloud Messaging (new)" for the new doc. Your suggestion for better naming is greatly appreciated.
lgtm with three caveats: Do we have to keep the old doc around, or can we just make this the new one? It's kind of weird, the 'new' around this doc. ** All references to Google APIs Console should be Google Developers Console (name changed recently). ** Make sure to test 'Register App or Extension' section against the new Google Developers Console. Some of the links/terminology may have changed.
On 2014/03/07 00:14:37, mkearney1 wrote: > lgtm with three caveats: > > Do we have to keep the old doc around, or can we just make this the new one? > It's kind of weird, the 'new' around this doc. Changed to keep the existing doc as main one and add a note pointing to the new one, per discussion. > > ** All references to Google APIs Console should be Google Developers Console > (name changed recently). > > ** Make sure to test 'Register App or Extension' section against the new Google > Developers Console. Some of the links/terminology may have changed. All checked. We're using the right terms.
The CQ bit was checked by jianli@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an LGTM may have been provided, it was from a non-committer or a lowly provisional committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
lgtm
rubberstamp lgtm
The CQ bit was checked by jianli@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jianli@chromium.org/145443002/320001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg
The CQ bit was checked by jianli@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jianli@chromium.org/145443002/320001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_compile_dbg
Message was sent while issue was closed.
Committed patchset #7 manually as r255671. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
