-
Notifications
You must be signed in to change notification settings - Fork 15
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: Fix Oasis CLI in sapphire-dev Docker image #488
Conversation
Building with |
Oh, OK, we can keep this workaround just here then. |
c5d4af6
to
8e62a74
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #488 +/- ##
=======================================
Coverage 61.86% 61.86%
=======================================
Files 37 37
Lines 3920 3920
=======================================
Hits 2425 2425
Misses 1288 1288
Partials 207 207 ☔ View full report in Codecov by Sentry. |
Do we need Oasis CLI to work in the |
I don't think many (any?) people are using the Emerald container, and if none of the CI tests are using it then I'd say no. Likewise with Ledger support, it's highly unlikely that Ledger will be used with the Oasis CLI within the sapphire-dev docker container. |
8e62a74
to
6b899c7
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.
This works fine :)
# /oasis --version
Software version: v0.7.1-gitbcbf421
Oasis SDK version: v0.7.1
Oasis Core version: v0.2300.9
Go toolchain version: go1.21.3
sapphire-dev local (oasis-core: 23.0.9, sapphire-paratime: 0.7.0-testnet, oasis-web3-gateway: 4.0.0-git6b899c7)
I guess we can do the version bump to Sapphire 0.7.0 (not testnet) too?
0779841
to
37e7a57
Compare
Fixes #487.
Note that in the long-term this should really be fixed in the CLI repo instead :)Apparently, this will break Ledger support, so we must keep this workaround just inside the Docker images, as they don't need Ledger support.