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

Hotfix: incorrect argument type in PoolCluster add method #1434

Merged

Conversation

claudio-at-float
Copy link
Contributor

@claudio-at-float claudio-at-float commented Oct 25, 2021

createPoolCluster method takes BasePoolCluster.PoolClusterOptions type as argument to create a pool cluster.

When adding nodes to the PoolCluster using the add method this parameter type is not correct since we need to pass options to define where nodes are and how to authenticate against them (host, port, etc.).
These options are defined in the PoolOptions type which extends the Connection.ConnectionOptions class.

@claudio-at-float claudio-at-float changed the title Hotfix: use PoolOptions type instead of PoolClusterOptions Hotfix: incorrect argument type in PoolCluster add method Oct 25, 2021
@sidorares
Copy link
Owner

Thanks @claudio-at-float

I'm currently not a PoolCluster + typescript user so ideally want someone using this features to review. @larshp maybe?

Also would be really great to add some initial types test harness ( maybe some happy path integration test written in TS )

@sidorares
Copy link
Owner

I think I need to disable coverage action for now, it's not working as intended

@larshp
Copy link
Contributor

larshp commented Oct 26, 2021

thanks for the heads up, I'll have a look sometime during the week, if not, feel free to ping me again 👍

@larshp
Copy link
Contributor

larshp commented Oct 28, 2021

add(), https://github.com/sidorares/node-mysql2/blob/master/lib/pool_cluster.js#L149, passes the config to new PoolConfig(config) which expects the fields from PoolOptions, looks good to me

@sidorares sidorares merged commit 1448ae8 into sidorares:master Nov 24, 2021
@claudio-at-float claudio-at-float deleted the fix/pool-cluster-options-add-types branch November 25, 2021 08:25
@santosh1997
Copy link

Hi @sidorares,
When will this be released? Any specific reason not releasing it till now?
I would be happy to help on it and it released

@sidorares
Copy link
Owner

@santosh1997 no particular reason, just no time to prepare the release ( somewhat manual process, partially because I want a bit of manual check partially because it's not automated yet )

you can help with automating - create a PR identical to sidorares/json-bigint#77

currently another manual step in addition to publishing is to check all merged PRs and add summary to readme. Would be cool to have a gh-actions workflow that acts as a chat bot and 1) comments with links to PRs merged since last release 2) allows ( potentially by someone in the pre-approved list ) respond with edited list and merge that into changelog and publish a release ( tag, npm push and github release with description, same as what was added to changelog )

@sidorares
Copy link
Owner

@santosh1997 I'll try to make a release tonight or at most this weekend. You can help by doing what imaginary automation workflow would do - check last release tag, verify what's latest on npm corresponds to it, prepare list of changes and comment here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants