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

Test: add version to managed pool. #2075

Closed
wants to merge 1 commit into from
Closed

Conversation

jubeira
Copy link
Contributor

@jubeira jubeira commented Nov 29, 2022

This PR showcases a possible version implementation for managed pool, refactoring as little as possible. Code is expected to build, but tests will fail.

Adding one more parameter to managed pool causes stack too deep errors (at managed pool, or at managed pool settings depending how the version is piped through). Working around it without refactoring seems not to be ideal; there might be a better solution than this one, but it'll probably still be messy.

Note: if ManagedPoolSettings implements Version, it also has a stack too deep error.

Given that the constructors are pretty much on the edge, I'd propose to add the version in the base layer and do a bigger refactor. Part of the issue is that we are mixing parameters that are needed in different layers in the same struct. One example can be seen here: NewPoolParams has name and version, which are not needed by ManagedPoolSettings. This can be solved in the refactor by grouping the parameters in a more appropriate way.

@jubeira jubeira added the do not merge Code playground or showcase - not intended to be merged label Nov 29, 2022
@jubeira
Copy link
Contributor Author

jubeira commented Nov 30, 2022

Closing in favor of either #2078 or #2080.

@jubeira jubeira closed this Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Code playground or showcase - not intended to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant