feat(wip): add depinject support for app v2 #3868
Conversation
crodriguezvega
changed the title
|
Will you want to keep an appv1 app.go around? I've been thinking about this lots while writing the upgrade... |
| // App Wiring Setup | ||
|
|
||
| func init() { | ||
| appmodule.Register(&modulev1.Module{}, |
There was a problem hiding this comment.
This call to Register complains with:
cannot use &modulev1.Module{} (value of type *"github.com/cosmos/ibc-go/v7/modules/core/module/v1".Module) as protoreflect.ProtoMessage value in argument to appmodule.Register: *"github.com/cosmos/ibc-go/v7/modules/core/module/v1".Module does not implement protoreflect.ProtoMessage (missing method ProtoReflect)compiler[InvalidIfaceAssign](https://pkg.go.dev/golang.org/x/tools/internal/typesinternal#InvalidIfaceAssign)
But I don't know why it doesn't implement protoreflect.ProtoMessage...
|
|
||
| package ibc.core.module.v1; | ||
|
|
||
| option go_package = "github.com/cosmos/ibc-go/v7/modules/core/module/v1"; |
There was a problem hiding this comment.
I see SDK has this api folder at the root where the .pb.go files are located. for now I put it in modules/core/module/v1. Looking for feedback where the best location would be.
I haven't given that much thought yet, or discussed with the rest of the team. I guess it doesn't hurt to keep both v1 and v2 for the time being? What pros/cons do you see of keeping both? |
|
Well, it seems like v2 is going to be much easier to work with. I suppose that if there are chains that wish to use appv1, then we should keep a simapp for them so that they can reference it. That said, I've noticed a lot of the work that I'm doing is abstracted away by v2. Should we open an issue? |
…-for-v2-apps # Conflicts: # modules/core/module.go
|
we should just use v2 IMO |
|
Kinda vibing this way, too, though I think ideal is using both :/ Maybe we follow the SDK's lead:
|
…-dependency-injection-for-v2-apps # Conflicts: # proto/ibc/core/module/v1/module.proto
…-dependency-injection-for-v2-apps # Conflicts: # go.mod
…-dependency-injection-for-v2-apps
|
I think I have made some progress with this, but I am going to close this PR and open a new one. |
crodriguezvega
deleted the
carlos/3560-support-sdk-dependency-injection-for-v2-apps
branch
Description
Still work in progress.
closes: #3560
Commit Message / Changelog Entry
imp: add depinject support for app v2see the guidelines for commit messages. (view raw markdown for examples)
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.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.