fix: PromisePoolCluster.of returns PromisePoolCluster instead of PoolNamespace

[jcmartineztiempo]

Fixes #3091.

[codecov]

Codecov Report

Attention: Patch coverage is 84.21053% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.98%. Comparing base (acba120) to head (1552305).
Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
lib/promise/pool_cluster.js	83.63%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3261      +/-   ##
==========================================
+ Coverage   88.77%   88.98%   +0.20%     
==========================================
  Files          85       86       +1     
  Lines       13448    13504      +56     
  Branches     1535     1560      +25     
==========================================
+ Hits        11939    12016      +77     
+ Misses       1509     1488      -21     

Flag	Coverage Δ
compression-0	88.98% <84.21%> (+0.20%)	⬆️
compression-1	88.98% <84.21%> (+0.20%)	⬆️
static-parser-0	86.55% <84.21%> (+0.21%)	⬆️
static-parser-1	87.37% <84.21%> (+0.20%)	⬆️
tls-0	88.39% <84.21%> (+0.20%)	⬆️
tls-1	88.75% <84.21%> (+0.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jcmartineztiempo]

Test added.

[wellwelwel]

Hey @jcmartineztiempo, thanks!
I couldn't pause to understand why the last test wouldn't finish, but it seems to have been fixed in recent commits.

Also, sorry for the lack of infos or support, December is usually very hectic 😅

[jcmartineztiempo]

Hi @wellwelwel,

Yeah...the last test wouldn't finish due a sql syntax error in mysql 5.7. I'll look into it later.

Don't worry about support...it's my first ever pool request!!

[wellwelwel]

Hi, @jcmartineztiempo, bringing this #3091 (comment) here:

Hi, when will it be merged and released?

Because of this, I figured that the work was not yet complete:

Yeah...the last test wouldn't finish due a sql syntax error in mysql 5.7. I'll look into it later.

Sorry for the miscommunication.

Could you rebase just to trigger the recent tests into your changes?

---

@sidorares, just a friendly ping: could you enable the following option in the repository settings?

• This will display a button that allows us to automatically rebase outdated branches without conflict and ensure that the test suite always runs through the latest version.

[jcmartineztiempo]

Hi,

just enabled.

Thanks

[wellwelwel]

Hi,

just enabled.

Thanks

Hey @jcmartineztiempo, this point is for @sidorares in MySQL2 repository 🙋🏻‍♂️

[sidorares]

@wellwelwel this is enabled now

[wellwelwel]

@jcmartineztiempo, sorry for the long delay. I've checked the behavior from mysqljs and if the typings match.

A concern was about a possible unintentional breaking change if someone checks the return using instanceof, but since it's the wrong behavior, it makes sense not to consider it that way.

A suggestion before merging (to keep consistency), would be to move the PromisePoolNamespace method to lib/promise/ and create a new file for it (pool_cluster.js) 🙋🏻‍♂️

[jcmartineztiempo]

Hi @wellwelwel,

I just refactor the code to move PromisePoolNamespace to lib/promise/pool_cluster.js.

[wellwelwel]

Thanks, @jcmartineztiempo (and welcome to the project contributors) 🚀

The fix is already available in 3.12.1-canary.be22202e version.

I've been holding the release for some time, because we depend on certain external factors. But you can follow the progress in #3377.