-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore: bring in github.com/orijtech/chdbg tool #14643
Conversation
CC @odeke-em |
Signed-off-by: Elias Naur <elias@orijtech.com>
b3af0c7
to
30d03c5
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.
Thank you @elias-orijtech, LGTM!
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.
utACK. This still needs to be done: https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#gomod and possibly configuring the docs. We can do that in a follow-up if you prefer.
|
||
require ( | ||
github.com/cosmos/cosmos-db v0.0.0-20221127110041-4564dc28e22b | ||
github.com/cosmos/iavl v0.19.2-0.20221208111139-992120de42e1 |
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.
Should this be bumped?
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
@@ -0,0 +1,28 @@ | |||
# chdbg |
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 should add some more information here on how to use it and read it. the below excerpt isn't very intuivutve since its hard to tell which store is the issue.
In the sdk we have a convention of providing a cli with helper text, should we adopt that here @julienrbrt ?
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.
Maybe all tools should use cobra yes.
} | ||
} | ||
|
||
func diff(dbdir1, dbdir2 string) error { |
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.
could we add some godocs to this file?
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.
needs a few changes before it can be merged
Let us know if you need a hand pushing this over the finish line 👌 |
In light of #13404 (comment) and orijtech/chdbg#1 (comment) I'm skeptical of pushing this PR through. Maybe just transfer the chdbg repository to Cosmos for now? Or, leave it as is until chdbg proves valuable for actual debugging use-cases. Also, according to Finally, I don't have the intimate knowledge of Cosmos to reproduce useful and typical debugging scenarios. For example, the notes at #13404 (comment) are helpful, but not enough for me. I suggest setting up a meeting with someone from Cosmos that can provide me with the necessary details for reproducing chain halts and what information would be useful. |
closing this PR as mentioned in the previous comment. We will keep the issue open and reassign it to someone on the team when there is time. |
No description provided.