minor channel fixes #8665
minor channel fixes #8665
Conversation
…rror to its own file
damiannolan
requested review from
aaronc,
alessio and
alexanderbez
as code owners
Codecov Report
@@ Coverage Diff @@
## master #8665 +/- ##
==========================================
- Coverage 61.39% 61.39% -0.01%
==========================================
Files 671 672 +1
Lines 38352 38343 -9
==========================================
- Hits 23548 23539 -9
Misses 12336 12336
Partials 2468 2468
|
|
The following merge checks have failed
++ @colin-axner |
|
@damiannolan could you add a change log? I think it would just be under "Improvements" but it would be good to specify that acknowledgement got moved out of the channel.proto file |
Sure no problem, will push an update. Edit: Done |
colin-axner
added
x/ibc
A:automerge
colin-axner
added
S:blocked
|
I'm in the process of preparing IBC to be migrated to a new repository (#8501). This requires removing IBC from the SDK (which I hope to do by the end of the week). I think it makes most sense just to migrate this pr to the new repository as well. Edit: Ah, wait, I don't see a "transfer pull request" button or rebasing to a new repository. Let's just hold for now. I'll either merge this before removing IBC or port the changes for you |
CHANGELOG.md
Outdated
| @@ -62,9 +62,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
| ### Improvements | |||
|
|
|||
| * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts | |||
| * (x/ibc) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Message `Acknowledgement` has been moved from channel.proto to acknowledgement.proto | |||
There was a problem hiding this comment.
Should we also move the PacketState struct out of channel.
Open to either moving to packet.proto or putting PacketState and Acknowledgement structs into one separate proto file since they're conceptually linked
There was a problem hiding this comment.
This seems like a question for the cosmos/ibc repo
| ) | ||
| registry.RegisterInterface( | ||
| "ibc.core.channel.v1.PacketI", | ||
| (*exported.PacketI)(nil), | ||
| ) | ||
| registry.RegisterImplementations( |
There was a problem hiding this comment.
Is this no longer needed @colin-axner ?
There was a problem hiding this comment.
Duplicate registration. RegisterInterface will register implementations as well
colin-axner
added
the
A:automerge
|
I reverted the proto changes since they weren't necessary and the acknowledgement proto type will be removed from the ibc module eventually. Lets get this merged now. Thanks @damiannolan |
|
Thanks @colin-axner! I may try to pick up some more issues in future to gain more knowledge of the codebase, hopefully relating to modules that are not in the process of being migrated! 😂 Cheers! |
Description
Completes the remaining items detailed in #7949. First issue so hoping process is covered here.
Running
make proto-allhas also performed regen on markdown docs.closes: #7949
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes