-
Notifications
You must be signed in to change notification settings - Fork 232
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
BREAKING CHANGE: remove @agoric/wallet-ui #6511
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.
- We plan to add it to wallet-app before merging this right?
- Will we move
web-components
to a separate repo too as part of pull web UI out of agoric-sdk #6474? - I wonder if there's now some things we can prune from yarn.lock. @mhofman might know better how to clean that up.
- I know @michaelfig was doing some stuff around ag-solo so I want to defer approval to him
Oh cleaning up |
It's already there, per the first line of the description. Specifically: https://github.com/Agoric/wallet-app/tree/master
That's not part of 6474. I'm not opposed but it would need some discussion as to where it goes and how it gets published.
Good call. I meant to include that and forgot. Here's my local,
18536 lines down to 13729, 26% reduction. I'll hold off on push until approval since that's liable to get stale as master changes.
Ok, I'll make him the sole reviewer. |
Oh okay I had trouble finding it. Why |
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.
LGTM. Can you please update yarn.lock
as well?
It's no longer a package. To use or develop the Wallet UI, see https://github.com/Agoric/wallet-app
43255b8
to
f1ce603
Compare
Any reason integration was bypassed for this? |
closes: #6474
Description
Per #6474, the Wallet app code is now at https://github.com/Agoric/wallet-app.
This PR removes it from agoric-sdk so that there is just one canonical Wallet UI. I believe that the instructions for using the older version compatible with ag-solo still work since they check out community-dev and not master. (Indeed, master wallet-ui is already broken for agsolo)
Security Considerations
Improves separation of builds and dependencies.
Documentation Considerations
End users are not affected.
Developers who wish to contribute to the Wallet app will need to know about the new repository. This commit message directs them.
Ag-solo users will need to use the instructions above to get an older version. (We've already decided agsolo doesn't need a GUI going forward).
Testing Considerations
CI