feat: enable working directory #15
Conversation
Codecov Report
@@ Coverage Diff @@
## main #15 +/- ##
==========================================
+ Coverage 93.38% 93.61% +0.22%
==========================================
Files 22 22
Lines 1043 1049 +6
==========================================
+ Hits 974 982 +8
+ Misses 52 51 -1
+ Partials 17 16 -1
Continue to review full report at Codecov.
|
|
@jmattheis Jannis, please take a look at this PR, share your thoughts! |
There was a problem hiding this comment.
Hey @nissim-natanov-outreach, thanks for this PR. See sub comments.
| @@ -17,6 +15,7 @@ import ( | |||
| type Config struct { | |||
| Name string | |||
| ExtendMethods []string | |||
| WorkingDir string | |||
There was a problem hiding this comment.
This PR enables working directory support for all packages.Load call. It is required when running unit tests in isolated directories with their own go.mod files - in this case the packages.Load call must be executed inside that directory.
Just to make clear, I understand this correctly. You have a repository with multiple go modules inside, and one goverter call should create converters for all these modules? Was it possible before this change to call goverter multiple times inside the directories of your go module?
There was a problem hiding this comment.
My use case is slightly different: unit tests. Our codegen tool wraps goverter codegen and invokes it as a library (not via cmd line). When our unit test runs, it creates a temporary directory, drops a go.mod with sources and invokes a codegen (and repeats it few times for different test cases). In each test case we need goverter to work in a scope of that temp (unit test) directory, and ignore the current dir.
P.S. I also noticed by removing go/importer usage the tool became a bit faster. I am not sure what was wrong with the go/importer (other than its lack of support for working dirs), but I did notice a big drop in exec time:
- with go/importer and unit test forced to do chdir before goverter: ~15-20s
- with current code: no need to do chdir: ~3-5s
There was a problem hiding this comment.
Thanks for the explanation, and the pref improvements are a good side effect.
|
comments addressed, PTAL |
This PR enables working directory support for all packages.Load call. It is required when running unit tests in isolated directories with their own go.mod files - in this case the packages.Load call must be executed inside that directory.
This PR preserves the original ParseDocs method signature for back compat, introducing new DocsParser to process the comments.
This PR also removes the use of go/importer package. It does not support the new go/modules well and both of its Import and ImportFrom methods are marked as deprecated. The golang.org/x/tools/go/packages is supposed to supersede the go/importer package, and it is already heavily used to parse docs and find convert methods, so there is no reason using the go/importer anymore. Moreover, there is no need to re-parse sources ... so I updated code to store the scope on the Converter itself.