-
Notifications
You must be signed in to change notification settings - Fork 229
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
fix!: separate wallet into User, Bridge, and Internal #2032
Conversation
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.
I really like where this is going. I think this needs a lot more comments inline in the code explaining how and where things are used. Additionally, I think the user facet should have a lot more. From the REPL or a deploy script, the user should be able to do things in code such as add a new issuer, change petnames, and so forth. These aren't (or at least) shouldn't be tightly coupled to the UI at all.
4b7d84d
to
45fd2b9
Compare
45fd2b9
to
479e991
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.
I really like the direction, but we still need more contextual comments. Not having contextual comments makes it very difficult for more than one developer to work on this code and drive it forward, and I think we shouldn't merge without comments explaining the design and next steps.
I've added a link in the description to #2042 which explains the architecture and general direction more. We can describe individual parts of the design in the current PR's code as necessary, but any high-level design should happen in #2042. |
e2e31a4
to
745cb57
Compare
This PR needs to address the discussion in #2058. |
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.
I think there's just one comment that I would take out about edgenames
but this looks like a great step forward to me. Thank you for doing this and for adding the detailed comments.
If it's ok with you, I think I would rather merge this in as a clean up and tackle the |
745cb57
to
815a876
Compare
Separation of APIs to simplify how to present them to users.
See #2042 for design goals and future direction.