-
Notifications
You must be signed in to change notification settings - Fork 606
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
improve getToDistributedCoinsFromGauges #2014
improve getToDistributedCoinsFromGauges #2014
Conversation
@@ -21,9 +21,14 @@ func (k Keeper) getDistributedCoinsFromGauges(gauges []types.Gauge) sdk.Coins { | |||
} | |||
|
|||
func (k Keeper) getToDistributeCoinsFromGauges(gauges []types.Gauge) sdk.Coins { | |||
// TODO: Consider optimizing this in the future to only require one iteration over all gauges. | |||
coins := k.getCoinsFromGauges(gauges) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (k Keeper) getCoinsFromGauges(gauges []types.Gauge) sdk.Coins {..}
is unused now, maybe you can remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, I'll remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once we figure out what to do with getCoinsFromGauges()
function and linter fix😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement with the complexity! LGTM!
Closes: #2005
What is the purpose of the change
Change
getToDistributeCoinsFromGauges
so that it only iterate through all gauges once.Testing and Verifying
This change is already covered by existing tests
Documentation and Release Note
Unreleased
section inCHANGELOG.md
? no