Closed wibblymat closed 8 years ago
Would it be worth changing this to just be steps to use rather than an explainer of the API?
I'd expect API docs to explain the API itself and the README be the rough steps to get this working.
install the library
npm install ....
Send a payload to a subscription.
// Subscription should be stored on your backend {endpoint: '.....', keys: {auth: '....', p256dh: '...'}}
sendWebPush(data, subscription);
Question regarding API - was there a reason for not handling the authentication header inside the library rather than the developer having to manage it?
addAuthToken
may be a bad name, then... it is the method you use to tell the library what your API key is for that service. You call it once in your application, not per message sent. So for GCM I'd do:
webpush.addAuthToken('.googleapis.com/gcm', '<string I copied from console.developers.google.com>');
subscriptions.forEach(sub => webpush.sendWebPush('whatever', subscription));
Does that make sense? The actual Authentication: key=BLAH
header is added to the POST request by the library.
Added a few comments, if you gave this to me today and said go, it would take me too long to figure out how to use it. I want a quick start explanation to get me going immediately, then go from there.
Ta for the explanation, still not entirely convinced by this API though.
1.) If it's only used by GCM, then should it just be of addGCMAuthToken('
webpush.addGCMAuthToken('
Essentially - I just really dislike the pattern requirement - feels to error prone and boilerplate.
SIDE NOTE: This should probably be moved to an issue and doesn't block these this docs PR.
+1 Peters comments.
I think keep this on the bare minimum titled getting started to just install and use the library for Chrome and Firefox.
We can either than add further comments in API docs OR seperate sections. For now I'd go minimal and start asking people for feedback and iterate on it.
Moved auth token comments to: https://github.com/GoogleChrome/push-encryption-node/issues/6 would appreciate any feedback.
@gauntface @petele I simplified the docs, removing the API reference.
I also simplified the auth token business - auth token is now a parameter of sendWebPush and we push the process onto the developer. This came up in the code review for the Go version, too, and in the end it seemed like it was actually easier for developers to grok what was going on if they were left in control.
LGTM
Commented on the naming of the module, but that can be moved to an issue.
R: @kaycebasques @gauntface CC: @petele