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

Expose Dialers inside Zk and Region #249

Merged
merged 5 commits into from
Apr 25, 2024
Merged

Conversation

mromaszewicz
Copy link
Contributor

Fixes #248

This change allows for overriding the default dialers used by the ZooKeeper client and the Region client to connect to their respective servers. Proxy-aware dialers can be installed by people who need them.

marcinromaszewicz added 2 commits April 25, 2024 10:04
This change allows for overriding the default dialers used by
the ZooKeeper client and the Region client to connect to their
respective servers. Proxy-aware dialers can be installed by people
who need them.
I didn't intend to change your import statements in my last commit
@dethi dethi requested a review from aaronbee April 25, 2024 18:47
Undid some more autoformatting changes after disabling the autoformatter.
Now my change should only contain the meaningful changes.
Copy link
Collaborator

@aaronbee aaronbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. This looks good, just a couple comments below on changing the API.

client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
Change dialers to be unnamed function types
@mromaszewicz
Copy link
Contributor Author

Requested changes made.

Copy link
Collaborator

@aaronbee aaronbee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@aaronbee
Copy link
Collaborator

We have a linter to prevent long lines:

./client.go:101: Line too long (134/100)
./client.go:281: Line too long (103/100)
./rpc_test.go:305: Line too long (144/100)
./zk/client.go:66: Line too long (132/100)
./zk/client.go:129: Line too long (106/100)
./mockrc_test.go:181: Line too long (149/100)

@mromaszewicz
Copy link
Contributor Author

We have a linter to prevent long lines:

./client.go:101: Line too long (134/100)
./client.go:281: Line too long (103/100)
./rpc_test.go:305: Line too long (144/100)
./zk/client.go:66: Line too long (132/100)
./zk/client.go:129: Line too long (106/100)
./mockrc_test.go:181: Line too long (149/100)

Yep, i saw the failures and fixed them right away. That anonymous function is very long :)

@aaronbee aaronbee merged commit fa78846 into tsuna:master Apr 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose Dialers for Zookeeper and Region package to allow using proxies
2 participants