[1/5] cluster: implement proper builder pattern & minor refactor #669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
utkarshgupta137
changed the title
jaymell
reviewed
Aug 24, 2022
jaymell
reviewed
Aug 24, 2022
jaymell
approved these changes
Aug 25, 2022
djc
reviewed
Aug 25, 2022
utkarshgupta137
force-pushed
the
cluster_builder
branch
2 times, most recently
from
9e3acae
to
1d04be5
Compare
djc
reviewed
Aug 31, 2022
|
This is much better! A few more minor changes. Please add a separate commit that bumps the MSRV to 1.59. |
utkarshgupta137
force-pushed
the
cluster_builder
branch
from
1d04be5
to
41cd862
Compare
MSRV change is entirely unrelated to this PR. I've created a separate PR: #673 |
utkarshgupta137
force-pushed
the
cluster_builder
branch
from
41cd862
to
72d5ee5
Compare
- move build method to builder - move open methods to the bottom
- implement proper builder pattern by adding `new` & `builder` methods to ClusterClient and `new` & `build` methods to ClusterClientBuilder - deprecate & redirect `open` methods for ClusterClient & ClusterClientBuilder
- this is to simplify passing multiple params to & inside ClusterConnection impl
utkarshgupta137
force-pushed
the
cluster_builder
branch
from
72d5ee5
to
92c30e6
Compare
djc
reviewed
Aug 31, 2022
utkarshgupta137
force-pushed
the
cluster_builder
branch
from
92c30e6
to
f3fd7d2
Compare
djc
approved these changes
Aug 31, 2022
|
Okay, so please rebase and self-review part 2 to have small commits and align with the other feedback I've given so far, this has taken a lot of time. |
|
Sure, waiting for you to merge this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most of the crates in the Rust ecosystem use
builder()&build()for client & builder as well asnew()for both of them. Currently, the redis crate uses the unintuitiveopen()methods for the same.With this PR, I've changed the cluster client to follow the proper builder pattern and redirect the old methods to the new ones.
Also refactored the cluster client in 2 main ways:
build()method from the client to the builder & made it publicClusterParamsto hold all the params inside a single struct instead of multiple fields in the main structIn addition, improved the documentation as I saw fit.