-
Notifications
You must be signed in to change notification settings - Fork 302
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
Summonerd: transition and export #3190
Conversation
eb22d74
to
218aa68
Compare
77f125e
to
8827a7d
Compare
8827a7d
to
872b716
Compare
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.
Look at this today and the diff itself LGTM, however for me the transition is not completing (ran for over an hour or so before stopping), I haven't debugged further yet but my suspicion is that it's to do with somewhere we're using the checked methods on CanonicalDeserialize
|
||
let phase1_crs = match storage.phase1_current_crs().await? { | ||
Some(x) => x, | ||
None => anyhow::bail!("Please run phase1 before this command 8^)"), |
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.
by phase1 here, we currently mean loading the generated phase 1 root using init
, and by phase2 we mean running start
(after transition
) ✅
More useful for error handling: this is because you can then react explicitly to a missing CRS, and warn to run other commands first.
872b716
to
ebc8234
Compare
Closes #3158.
Closes #3159.
Closes #3160.
We could do some integration testing of things before merging as well.