-
Notifications
You must be signed in to change notification settings - Fork 8
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: update for API route change #47
Conversation
TODO:
|
Hey @fubuloubu I believe we still have issues, here is what it is throwing
And here is the versions installed
|
@solarthesis confirmed this locally just using |
@fubuloubu believe there are still things i'ts not liking
|
Yeah was just saying I got the same issue that way haha, working on debugging |
@solarthesis should be fixed now! |
ok let me test it out, 1 second |
Works perfectly @fubuloubu |
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.
couple o' nits / small-refactor opportunities
""" | ||
View and filter all transactions for a given Safe using Safe API | ||
""" | ||
if account in cli_ctx.account_manager.aliases: |
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 logic seems argument-callback worthy
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.
Yes but I think could be upstreamed to ape.cli
@Uxio0 please take note of breaking changes on gnosis api, which is causing issues for integrations like this safe-global/safe-transaction-service@6753c7a |
What I did
just changed the API route
fixes: #46
How I did it
How to verify it
Checklist