Skip to content

Conversation

ealain
Copy link
Contributor

@ealain ealain commented Jul 2, 2018

I removed a few spurious words from a paragraph in Readme.md. I assume these were inadvertently left in a previous change.

Copy link
Member

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

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

Yea, good catch. But as I am reading both versions, neither of them make any sense.

@ealain
Copy link
Contributor Author

ealain commented Jul 3, 2018

Do you think the below wording would be acceptable?

The end method takes an optional callback that you can use to know when
all the connections are ended.
Once pool.end is called, pool.getConnection and other operations
can no longer be performed.
Wait until all connections in the pool are
released before calling pool.end. If using the shortcut method
pool.query, wait until it completes.
pool.end calls connection.end on every active connection in the pool.
This queues a QUIT packet on the connection and sets a flag to prevent
pool.getConnection from creating new connections. All commands / queries
already in progress will complete, but new commands won't execute.

@dougwilson dougwilson added the docs label Jul 3, 2018
@dougwilson
Copy link
Member

So it looks that that is meant to replace more than the one paragraph updated in the PR? Can you either (a) update the PR so I can tell what exactly this is replacing or (b) write in the comments here the section that that wording would replace?

Copy link
Member

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

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

Just the one comment, otherwise LGTM. Please post a comment when you update the PR with a change, though, as otherwise I don't know when you push a new change.

since they have a pending `QUIT` packet in their queue; wait until releasing
all connections back to the pool before calling `pool.end()`.

Since the `pool.query` method is a short-hand for the `pool.getConnection` ->
Copy link
Member

Choose a reason for hiding this comment

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

Can you find a spot to keep the note about it being a short-hand for the three methods? A user previously added that part to this section because they were confused without it. I would like to keep it there for them if possible.

@ealain
Copy link
Contributor Author

ealain commented Jul 5, 2018

Indeed, it makes sense to add this reminder here.

@dougwilson dougwilson merged commit bc9cd35 into mysqljs:master Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants