refactor: cat replenish polish #14
Merged
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
PaulBernier
commented
Feb 10, 2020
controller_cat.go
Outdated
|
|
||
| if uint(c.peers.Total()) >= c.net.conf.Target { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| connect = nil |
There was a problem hiding this comment.
I was wondering if this nil assignment is actually necessary
WhoSoup
approved these changes
Feb 11, 2020
There was a problem hiding this comment.
Spent sometime carefully reading the catReplenish function as it looks a pretty important functions.
This function is already the third or fourth rework. I constantly feel like there's an easier way to do it all but I can't think of one, You not finding anything substantially wrong with it makes me feel better about it.
I like these changes.
controller_cat.go
Outdated
|
|
||
| if uint(c.peers.Total()) >= c.net.conf.Target { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| connect = nil |
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.
Spent sometime carefully reading the catReplenish function as it looks a pretty important functions. Only found some minor polish details that help the readability and comprehension, to me at least, so we can discuss it.
denyforcanDial. It allows to remove some negation in condition and have condition readif canDial then add to the list of endpoints to try to connect toinstead of having a guard statement with immediate continue. Matter of style but, I think it's slightly better...minReseeddeclaration happening before the special peers handling while not being until after it. (same for the declaration ofattemptsLimit)