fix(cosmosanalysis/module): discover proto files only in certain dirs #1389
Conversation
| return err | ||
| } | ||
| modules, err := g.discoverModules(path) | ||
| modules, err := g.discoverModules(path, "") |
There was a problem hiding this comment.
we should also pass it for dependencies as they can contain invalid proto files for testing or other purposes
There was a problem hiding this comment.
But each dependency can have a proto file in a different place, right? How can we handle it?
There was a problem hiding this comment.
that's right. this can be also configured in the config.yml per dependency basis with a default proto/. but this might be too much for now. how about if we pass some known patterns to ignore here? Starting from the ethermint example, we can make it ignore **/test/** and **/tests/**.
There was a problem hiding this comment.
I like the pattern idea. It seems awesome. If we move the build.proto.path to a list, we can also define paths to skip, like ["proto/**, !proto/node_modules"]
Let's make this PR simple for a quick fix for the v0.17.1 and create an issue in the repo. What do you think?
ilgooz
changed the title
build.proto.path
ilgooz
changed the title
| args args | ||
| want []Module | ||
| }{ | ||
| { |
There was a problem hiding this comment.
can we add a test case here to actually compare a valid []Module value returned after module discovery? but not required for this PR I think.
There was a problem hiding this comment.
The first two cases return the Module from the testdata project. But both have all empty parameters and no modules from 3trd part packages. I cannot think of a good way to do it without creating an entire chain scaffold into the testdata only for this test. maybe in the future we can create a common/global testdata folder for all packages with a scaffold chain for we can perform these tests
…ignite#1389) * avoid search proto files outside the proto folder defined into the config * fix unit tests * improve tests * add unit test and testdata * remove unused error assertion * add relative testdata path * remove nil variable parameter from tests Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
closes #1342
What does this MR does?
build.proto.path; instead, compile all proto files from the project.Changes
How to test?