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

Refactor ledger startup code #1231

Closed
wants to merge 36 commits into from
Closed

Conversation

sug0
Copy link

@sug0 sug0 commented Jul 11, 2022

I have broken down the run_aux function into several different logical units, which should make the ledger startup code more modular and easier to reason about.

This PR will help with the changes done to the eth bridge, since the latter will require even more startup logic code to manage geth. For the same reason, it is in conflict with #1196, so this will need to be handled.

@sug0 sug0 requested a review from batconjurer July 11, 2022 12:50
@sug0
Copy link
Author

sug0 commented Jul 11, 2022

CI timed out :\

+ sccache --start-server
sccache: Starting the server...
sccache: error: Timed out waiting for server startup

@batconjurer batconjurer requested a review from tzemanovic July 11, 2022 14:25
Copy link
Member

@batconjurer batconjurer left a comment

Choose a reason for hiding this comment

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

Looks great to me. Just a couple of tiny things

apps/src/lib/node/ledger/mod.rs Outdated Show resolved Hide resolved
apps/src/lib/node/ledger/mod.rs Show resolved Hide resolved
Copy link
Member

@tzemanovic tzemanovic left a comment

Choose a reason for hiding this comment

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

Nice refactor, thanks!

@tzemanovic
Copy link
Member

I tried to check the windows compilation, but unfortunately, we have a unix only dependency in the wallet file-lock, so it's not working atm. We don't officially support windows yet, so it's fine to go ahead - visually it seems fine to me :)

@tzemanovic
Copy link
Member

@sug0
Copy link
Author

sug0 commented Jul 13, 2022

done! added a changelog entry

@juped
Copy link
Member

juped commented Aug 3, 2022

Moved to anoma/namada#270 anoma/namada#271

@juped juped closed this Aug 3, 2022
@mariari mariari deleted the tiago/refactor-ledger-run-with-cleanup branch October 9, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants