-
Notifications
You must be signed in to change notification settings - Fork 60
Various doc improvements #374
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome PR! Thanks for addressing these old doc mistakes! I have some suggestions but feel free to accept or ignore.
src/commands/push.js
Outdated
@@ -37,7 +37,9 @@ A shortcut for \`zapier build && zapier upload\` - this is our recommended way t | |||
|
|||
> Note: this is always a safe operation as live/production apps are protected from pushes. You must use \`zapier promote\` or \`zapier migrate\` to impact live users. | |||
|
|||
If you have not yet registered your app, this command will prompt you for your app title and register the app. | |||
> Note: this will overwrite the version specified in your \`package.json\`. If you want to push to a new version, increment the "version" key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be more specific by saying
..., increment the "version" key in
package.json
.
README-source.md
Outdated
@@ -687,7 +686,7 @@ module.exports = { | |||
|
|||
|
|||
### `bundle.cleanedRequest` | |||
> `bundle.cleanedRequest` is only available in the `perform` for web hooks and `getAccessToken` for oauth authentication methods | |||
> `bundle.cleanedRequest` is only available in the `perform` for webhooks and `getAccessToken` for oauth authentication methods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a period at the end of the sentence? The same goes other similar places in this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked them all! They all end in punctuation now.
README-source.md
Outdated
|
||
> `bundle.targetUrl` is only available in the `performSubscribe` and `performUnsubscribe` methods for webhooks | ||
|
||
This the url to which you should send hook data. It'll look something like `https://hooks.zapier.com/1234/abcd`. We provide it so you can make some sort of POST request to your server and store this as a destination for new info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with both url
and URL
, but we should be consistent across the entire document. So I think we should be using URL
here.
Also, I'd suggest editing this:
We provide it so you can make some sort of POST request to your server and store this as a destination for new info.
to:
We provide it so we can make a POST request to your server, and your server should store this URL to hit when there's new data.
Various minor doc improvements.
fixes #356, fixes #141, fixes #353, fixes #91
Note I purposefully didn't build docs for the PR because it adds a lot of noise. I'll do that before I merge.