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

use master branch of cosmos/ledger-cosmos-go #1801

Closed
wants to merge 4 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Oct 11, 2022

While working on Akash today I noticed that there were some very
strange
things going on with our ledger imports. The tl;dr is:

  • we should stop maintaining cosmos/ledger-go
  • we should ensure that we routinely cut releases of
    cosmos/ledger-go-cosmos

Here's a PR that gets Gaia on the right track, reflecting those changes.

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #1801 (94b6955) into main (4f93f23) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1801   +/-   ##
=======================================
  Coverage   43.19%   43.19%           
=======================================
  Files          15       15           
  Lines        1338     1338           
=======================================
  Hits          578      578           
  Misses        740      740           
  Partials       20       20           

@okwme
Copy link
Contributor

okwme commented Oct 13, 2022

Thanks @faddat .
Zondax maintains both of those repositories.
It was relatively recently that we updated to their latest release (#1576, #1634).

We need to confirm whether there are API breaking changes that would impact users of gaia to ensure they're included in communications. Have you investigated what are the breaking changes between the updates you've included?
If not, maybe someone from Zondax can help us review this PR—maybe @philipglazman or @jleni?

@jleni
Copy link
Member

jleni commented Oct 14, 2022

Hi @faddat @okwme
As Billy explained, we have recently taken over the maintenance in this org and we are moving to a more predictable release schedule.

Could you provide some more info about these "strange things" you describe? Or it is related only to go.mod issues and a release would clear up the problem?

@jleni
Copy link
Member

jleni commented Oct 14, 2022

I have just created this issue cosmos/ledger-cosmos-go#30 to track the repo reorganization that will take place soon

@@ -256,6 +255,7 @@ require (
github.com/yagipy/maintidx v1.0.0 // indirect
github.com/yeya24/promlinter v0.2.0 // indirect
github.com/zondax/hid v0.9.1-0.20220302062450-5552068d2266 // indirect
github.com/zondax/ledger-go v0.12.2 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

The zondax/ledger-go seems not well maintained. I think we should avoid importing this package into Gaia

@okwme
Copy link
Contributor

okwme commented Nov 1, 2022

thanks for the link to track progress @jleni !
We'll close this PR until the zondax plan for maintained libraries is complete and reference them directly then

@okwme okwme closed this Nov 1, 2022
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