Deprecated Pool.end to Pool.release

[jbrooksuk]

This is better.

[jbrooksuk]

Just realised I forgot to add a test to ensure that release and end work. I'll add that now.

[jbrooksuk]

I've updated all of the pool tests, and also added test-connection-end to ensure that end still works as it should for now.

[felixge]

LGTM. Anybody else who can review? cc @dresende @NateLillich ...

[jbrooksuk]

Looks I like broke the tests, https://travis-ci.org/felixge/node-mysql/jobs/8197741

I thought:

pool.getConnection(function(err, connection) {
  if (err) throw err;
  pool.getConnection(function(err) {
    assert.ok(err);

    var shouldGetConnection        = false;
    pool.config.waitForConnections = true;
    pool.getConnection(function(err, connection2) {
      if (err) throw err;
      assert.ok(shouldGetConnection);
      assert.strictEqual(connection, connection2);

      pool.end();
    });

    shouldGetConnection = true;
    connection.release();
  });
});

Would work?

[tellnes]

Pool#release != pooled Connection#release
Take a look at the Pool#_createConnection implementation.

I could probably make a pull request if you don't want to fix it up @jbrooksuk?

[jbrooksuk]

Ah balls, so I modified the wrong pool?

[jbrooksuk]

What I should've modified was https://github.com/felixge/node-mysql/blob/master/lib/Pool.js#L147-151 ?

[tellnes]

@jbrooksuk yes. And you need to cleanup in Pool#_removeConnection also.

I think we should make the end method only print a deprecation warning and in the next version restore its default behavior.

[jbrooksuk]

Okay, so if I go through and rename .end in regards to actual pool Connection#end do you want .end to not be an alias? Just simply warn that it's deprecated?

And could you quickly explain what you mean by tidy Pool#_removeConnection? I see it's using _realEnd etc, those will be renamed too right?

[tellnes]

The end method should print a warning and then call the release method. In the next version we should remove the custom end function so it will get back it's default behavior.

In Pool#_removeConnection you should cleanup the connection object like it was never used in the pool. Thus restor the orginal end method (like it does today) and remove the release method.

[dresende]

Yes, you should just rename .end to .release and add an .end that would throw the warning and call .release. (not actually throw, write to stderr..)