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

ledger: fix error shadowing in onlineAccountsNewRoundImpl #5188

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Mar 9, 2023

Summary

Fix err shadowing introduced in #5177
Also add a small refactoring around Ledger and SigLedger initialization

Test Plan

Added new test

@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #5188 (26bd1d7) into master (bee25d7) will increase coverage by 0.01%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master    #5188      +/-   ##
==========================================
+ Coverage   53.52%   53.53%   +0.01%     
==========================================
  Files         439      439              
  Lines       54985    54993       +8     
==========================================
+ Hits        29429    29443      +14     
+ Misses      23270    23262       -8     
- Partials     2286     2288       +2     
Impacted Files Coverage Δ
data/transactions/logic/eval.go 90.25% <0.00%> (-0.07%) ⬇️
ledger/acctonline.go 77.66% <ø> (ø)
ledger/acctdeltas.go 79.20% <100.00%> (+0.11%) ⬆️
ledger/internal/appcow.go 74.92% <100.00%> (+0.35%) ⬆️

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algorandskiy algorandskiy changed the title WIP: ledger: fix error shadowing in onlineAccountsNewRoundImpl ledger: fix error shadowing in onlineAccountsNewRoundImpl Mar 9, 2023
jannotti
jannotti previously approved these changes Mar 10, 2023
Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

Slight comment adjustment - not terribly important.

ledger/internal/appcow.go Outdated Show resolved Hide resolved
algorandskiy and others added 2 commits March 10, 2023 10:54
Co-authored-by: John Jannotti <jannotti@gmail.com>
Copy link
Contributor

@bbroder-algo bbroder-algo left a comment

Choose a reason for hiding this comment

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

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants