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: move accountUpdatesLedgerEvaluator to tracker.go #5644

Merged

Conversation

algorandskiy
Copy link
Contributor

Summary

accountUpdatesLedgerEvaluator used to only implement a couple lookup methods from accountUpdates but now it uses txtail and online accounts trackers. Moved to tracker.go where it is used.

Test Plan

Existing tests should pass

Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

Could you wait to do this until we've deprecated the version of Indexer that depends on go-algorand? It uses these functions.

Nevermind. They're still in the same package so it shouldn't make a difference.

@winder winder self-requested a review August 8, 2023 18:10
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #5644 (be31006) into master (39ad943) will increase coverage by 0.28%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #5644      +/-   ##
==========================================
+ Coverage   54.63%   54.92%   +0.28%     
==========================================
  Files         463      463              
  Lines       64553    64553              
==========================================
+ Hits        35270    35457     +187     
+ Misses      26879    26709     -170     
+ Partials     2404     2387      -17     
Files Changed Coverage Δ
ledger/acctupdates.go 71.82% <ø> (+0.46%) ⬆️
ledger/tracker.go 73.76% <60.00%> (-3.06%) ⬇️

... and 34 files with indirect coverage changes

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

Copy link
Contributor

@AlgoAxel AlgoAxel left a comment

Choose a reason for hiding this comment

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

Difficult to tell with the way git renders it, but reading over this it appears to be pure drag-and-drop reorganization. Cleans up our imports slightly too :)

@algorandskiy algorandskiy merged commit 0e651fd into algorand:master Aug 8, 2023
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.

3 participants