-
Couldn't load subscription status.
- Fork 570
feat(pkg/cosmosanalysis): support discovery of modules defined in a variable #2820
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
Conversation
|
Visit the preview URL for this PR (updated for commit bb4ed5e): https://ignite-go-docs--pr2820-feat-cosmosanalysis-mv8tql5g.web.app (expires Tue, 20 Sep 2022 09:46:09 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
The changeset resolves the modules defined in a variable within the same package but in a different file.
bb4ed5e to
bf74815
Compare
|
Getting this kind of errors :
According to this: google/protobuf-gradle-plugin#405 (comment) it's because we're including the same proto files multiple times somewhere |
@clockworkgr how can I replicate it? |
clone latest osmosis...run ignite g ts-client |
It is working for me with c77822b which is the latest change in the |
is it generating all used modules for you? i dont see auth/bank/staking etc in my output |
I have to take a look at it Update: I have checked different cases and the modules that are failing are described in this comment. |
Some apps define the Go import path in the proto files without the version suffix while the Go module uses a version suffix. This change allows the custom module discovery to work in such case.
Changeset modifies the module discovery to search inside custom module folders for the RPC services implementation.
The change allows discovery for blockchain apps that are versioned. It also handles the cases where custom app modules are imported using a Go import that might or might not contain the version suffix.
The protp package is required to generate code for other blockchains like Gaia, otherwise generation would fail because the proto option `cosmos_proto.scalar` won't be available while generating the typescript code for some cosmos SDK modules. Cosmos SDK removed the `cosmos_proto` from its third party proto includes in changeset cosmos/cosmos-sdk@0cb7fd0
|
With the latest changes I got to a point where:
Akash fails because the way the modules are defined which is not the usual way and it could be handled by the CLI. Regen have an issue because of a Cosmos ORM proto file imported in the Crescent have an issue with the Regarding Gaia "working" before what was working was the generation of third party modules while the custom ones were not discovered, after one of the changes the custom modules were discovered and the generation was failing because the |
Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com>
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.
Can we add some tests for this functionality in this or another PR?
Which functionality? If it's for the new module discovery cases they are tested. |
We have added a lot of untested utility functions in |
This case discover registered modules registered as NewBasicManager function arguments.
…very' into feat/cosmosanalysis-module-discovery
Blockchain apps are versioned when the import path in the `go.mod` file contains a version suffix.
…ariable (ignite#2820) * feat: change module discovery to resolve modules from a variable * feat: change module discovery to resolve modules from a pkg variable The changeset resolves the modules defined in a variable within the same package but in a different file. * feat: change module discovery to resolve modules from a other pkg variable * feat: discover app modules created using a function call * refactor: simplify module discovery * chore: update changelog * chore: minor code improvements and documentation * fix: cosmosanalysis to work when proto Go import is missing version Some apps define the Go import path in the proto files without the version suffix while the Go module uses a version suffix. This change allows the custom module discovery to work in such case. * refactor: improve discovery for different custom module layouts Changeset modifies the module discovery to search inside custom module folders for the RPC services implementation. * fix: remove default from typescript client generate output flag * refactor: improve module discovery to work with versioned Go modules The change allows discovery for blockchain apps that are versioned. It also handles the cases where custom app modules are imported using a Go import that might or might not contain the version suffix. * feat: add `cosmos_proto` to protoc includes The protp package is required to generate code for other blockchains like Gaia, otherwise generation would fail because the proto option `cosmos_proto.scalar` won't be available while generating the typescript code for some cosmos SDK modules. Cosmos SDK removed the `cosmos_proto` from its third party proto includes in changeset cosmos/cosmos-sdk@0cb7fd0 * feat: add error support to `NewBasicManager` call discovery * review: add comment to basic manager discovery Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com> * chore: fix syntax * fix CL * test: registered modules for osmosis * test: add test for standard module discovery cases This case discover registered modules registered as NewBasicManager function arguments. * test: add test for module discovery with API routes * tests: add tests for `cosmosanalysis` module helper functions * tests: add discovery test for versioned apps Blockchain apps are versioned when the import path in the `go.mod` file contains a version suffix. * test: registered modules for current spn, gaia and juno Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com> Co-authored-by: Thomas Bruyelle <thomas.bruyelle@tendermint.com> Co-authored-by: Alex Johnson <alex@shmeeload.xyz>
Closes #2818