-
Notifications
You must be signed in to change notification settings - Fork 524
Enable ineffassign linter and resolve its errors #2574
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,10 +153,7 @@ func computeAccountMinBalance(client libgoal.Client, cfg PpConfig) (requiredBala | |
| fmt.Printf("required min balance for app accounts: %d\n", requiredBalance) | ||
| return | ||
| } | ||
| var fee uint64 = 1000 | ||
| if cfg.MinFee > fee { | ||
| fee = cfg.MinFee | ||
| } | ||
| var fee uint64 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. while this code is technically correct, I think that we shouldn't do that as is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I left a comment here https://github.com/algorand/go-algorand/pull/1715/files#r671536585 about whether this was intentional or not ... I wasn't sure if this uncovered a bug or need to fix/clarify something like you're saying.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pinging @algorandskiy for his opinion since these lines here were made ineffectual by #1715
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A note about using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh OK, I see so MinFee is still used in pingpong.go:WorkerState.fee(). I just wanted to make sure you realized these lines 156-159 being removed above were ineffectual (1000 is unused, and fee = cfg.MinFee is always overwritten). |
||
| if cfg.MaxFee != 0 { | ||
| fee = cfg.MaxFee | ||
| } else { | ||
|
|
||
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.
@tsachiherman this is the most significant issue I found — before this change, the value of
err = s.ledger.AddValidatedBlock(*vb, *cert)on line 319/320 is ineffectual, and has no impact later on line 324/325 where it checksif err != nil