Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update jsdocs for createConnection #6766

Merged
merged 1 commit into from Jul 25, 2018
Merged

update jsdocs for createConnection #6766

merged 1 commit into from Jul 25, 2018

Conversation

lineus
Copy link
Collaborator

@lineus lineus commented Jul 24, 2018

per my comment on #6756 this change removes the old parameter signature from the createConnection jsdoc example.

I'm also thinking we could change line 420 in connection.js from

  if (options) {

to

  if (options && typeof options === 'object') {

but I'm not sure if we should throw an error instead if options isn't an object? @vkarpov15, what do you think?

all tests pass.

@vkarpov15 vkarpov15 added this to the 5.2.6 milestone Jul 25, 2018
@vkarpov15 vkarpov15 added the docs This issue is due to a mistake or omission in the mongoosejs.com documentation label Jul 25, 2018
@vkarpov15 vkarpov15 removed this from the 5.2.6 milestone Jul 25, 2018
@vkarpov15
Copy link
Collaborator

We should throw an error if options isn't an object. Cleaner developer experience that way because people might stumble across the old syntax like the poster of #6756 did. Feel free to put in a PR for that.

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Thanks!

@vkarpov15 vkarpov15 merged commit 0992629 into Automattic:master Jul 25, 2018
@vkarpov15 vkarpov15 added this to the 5.2.6 milestone Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants