Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Conversation

@smithsz
Copy link
Contributor

@smithsz smithsz commented Apr 27, 2018

Thanks for your hard work, please ensure all items are complete before opening.

What

Add 'Callback Signature' section to README.

Issues

Fixes #303.

Copy link
Member

@ricellis ricellis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 minor comments


After initialization, in general, callback functions receive three arguments:
### Callback Signature

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is in the examples later anyway, but I think it would be nice to have the function signature here too

function(err, body, headers) {}

just so people can actually see the signature before the description of the parameters

README.md Outdated
[ '_replicator', '_users' ]
```

* `header` - The HTTP _response header_ (if no error). For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems weird that we call this header and not headers since it is all the response headers. Even if they are in a single object. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed. I'll update all the instances in the README.

@smithsz smithsz force-pushed the 303-document-error branch from 6969032 to a11cee6 Compare April 27, 2018 13:35
@smithsz smithsz force-pushed the 303-document-error branch from a11cee6 to 65832ab Compare April 30, 2018 10:41
@smithsz smithsz merged commit bce02b5 into master Apr 30, 2018
@smithsz smithsz deleted the 303-document-error branch April 30, 2018 12:40
@ricellis ricellis added this to the 2.2.0 milestone May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants