Standardized action verbs on remaining libraries #184
Conversation
| @@ -4,7 +4,8 @@ var key = 'YOURAPIKEY' | |||
| , SparkPost = require('sparkpost') | |||
| , client = new SparkPost(key); | |||
|
|
|||
| client.transmissions.find('YOUR-TRANsMISSION-KEY') | |||
There was a problem hiding this comment.
Yeah... I fixed a lot of weird typos.. like tranmission
| @@ -1,33 +1,22 @@ | |||
| 'use strict'; | |||
|
|
|||
| var api = 'message-events'; | |||
| const api = 'message-events'; | |||
There was a problem hiding this comment.
are we dropping node 0.12 support? I think I remember that discussion somewhere, but just wanted to make sure.
There was a problem hiding this comment.
That is correct. We are dropping support for 0.10 and 0.12. If they are still using those versions, they can use version 1.x of the library.
| messageEvents.search(options, function(err, data) { | ||
| Object.keys(options).forEach(function(key) { | ||
| expect(client.get.firstCall.args[0].qs).to.have.property(key).and.equal(options[key]); | ||
| messageEvents.search(options) |
There was a problem hiding this comment.
you'll need to return this promise as well to tell mocha to wait for the test, otherwise it will move on before the test completes
| if (Array.isArray(opt)) { | ||
| expect(firstCallQS).to.have.property(key).and.equal(opt.toString()); | ||
| } | ||
| messageEvents.search(arroptions) |
| var opt = arroptions[key] | ||
| , firstCallQS = client.get.firstCall.args[0].qs; | ||
| if (Array.isArray(opt)) { | ||
| tests.push(expect(firstCallQS).to.have.property(key).and.equal(opt.toString())); |
There was a problem hiding this comment.
these are synchronous expectations, so no need to wrap them in the promise later. Expect will throw an error if the assertion fails, and that will result in a rejected promise since we are inside a then callback`.
| @@ -4,6 +4,18 @@ var key = 'YOURAPIKEY' | |||
| , SparkPost = require('sparkpost') | |||
| , client = new SparkPost(key); | |||
|
|
|||
| // Promise | |||
| client.messageEvents.search({}) | |||
There was a problem hiding this comment.
technically this returns 1000 events for the last hour 🤓
There was a problem hiding this comment.
I just refactored the example. We can add a comment, change the name of the file, etc. Open to suggestions.
There was a problem hiding this comment.
And that's not in the API docs.
|
|
||
| // Promise | ||
| client.messageEvents.search(searchParams) | ||
| .then((err, data) => { |
There was a problem hiding this comment.
these promise callbacks shouldn't take an err object :) I just spent a few minutes trying to get this example to work before I realized what was happening
|
|
||
| // Promise | ||
| client.messageEvents.search({}) | ||
| .then((err, data) => { |
|
|
||
| // Promise | ||
| client.messageEvents.search(searchParams) | ||
| .then((err, data) => { |
|
lgtm |
This PR should finish and close issue #175.