-
Notifications
You must be signed in to change notification settings - Fork 556
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
[2/5] [4] cluster: change order of codes #709
Conversation
6786bf8
to
45f31cd
Compare
I rebase it because of PR 3 is merged. After this PR, I'll create PR one by one. |
d862167
to
ac05805
Compare
ci problem is same with previous intermittent one. #704 (comment) |
ac05805
to
fb2118a
Compare
IMO, it is better to define everything before it is used in the code, to the extent possible. The purpose of the original commit was to bring the struct & impls of ClusterConnection next to each other. You're of course, free to follow your convention. |
Not to mention that the more changes you make early on, the harder it will be to apply future changes. There are a lot of changes in this file, so it would probably be best not to mess with the order of code if it doesn't make a lot of difference. I apologise for moving a lot of code in the first place. |
@utkarshgupta137 I think some changes are can't be avoidable and rebase difficulty is still bearable. So I'll be careful more not to do mis-rebase for now. But your concern is reasonable too. I'll find other better way to satisfy ease-of-rebase and code-change-ability when I can't bearable more. |
…ype, struct, enum, fn cluster: move main struct to above of main impl move methods in order: constructor, pub fn, pub(crate) fn, fn
fb2118a
to
8001465
Compare
Changes
Effect
Note