Add gocp tool #275
Add gocp tool #275
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
==========================================
- Coverage 58.95% 58.69% -0.26%
==========================================
Files 44 46 +2
Lines 1988 2046 +58
==========================================
+ Hits 1172 1201 +29
- Misses 686 708 +22
- Partials 130 137 +7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Remove gocpy binary Fix typos Add tests Apply suggestions from code review Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Done. |
| "fmt" | ||
| "os" | ||
|
|
||
| flag "github.com/spf13/pflag" |
There was a problem hiding this comment.
Only because this package is used in all other tools. Indirectly in most of them via spf13/cobra and directly in crosslilnk and semconvgen. flag is not directly used in any of the tools.
| for scanner.Scan() { | ||
| txt := scanner.Text() | ||
|
|
||
| if !pkgReplaced && strings.HasPrefix(txt, "package ") { |
There was a problem hiding this comment.
This seems like it has limited functionality compared to using a text template. When these copied internal packages start to depend on each other the imports will need to be appropriately templatized. Why not just use a text template for this?
There was a problem hiding this comment.
When these copied internal packages start to depend on each other the imports will need to be appropriately templatized.
That is true. Currently, we already have a use case for it: https://github.com/open-telemetry/opentelemetry-go/blob/4ccd59056982f34c3fcc8faef574f209b70d0f59/sdk/internal/env/env_test.go#L24
Why not just use a text template for this?
I found it easier to use actual Go code than writing Go source code templates because we get IDE support.
The second reason is that the the "shared Go code" is analyzed using golangci-lint what we could not say about templates. The problem is that if the generated code has // Code generated by gocp. DO NOT EDIT. comment then it won't be analyzed by golangci-lint. We could alter the text to something like // Generated by gocp. However, we will lint all the generated files instead of only linting the source files .
Summary
Having all that said I feel that we indeed need this flexibility and I will try to simply create a simple CLI for text/template. Probably the tool should be also renamed to something more or less like gotmpl.
|
Closing per #275 (comment). I will create a new PR (next iteration) in the following days. |
Why
Fixes #276
Towards addressing open-telemetry/opentelemetry-go#3846
Towards fixing open-telemetry/opentelemetry-go#3548
Addresses open-telemetry/opentelemetry-go#3841 (comment)
What
Add
gocpytool that can be used for sharing internal code (more inREADME.md).This tool is currently needed for https://github.com/open-telemetry/opentelemetry-go.
Prior art (with example usage): open-telemetry/opentelemetry-go#3841
Other
Do we want to use it for
internal/repoandinternal/syncerror? Personally, I think there is no big need as these APIs looks pretty stable. And AFAIK the tools are bumped together. On the other hand, it may be good to dogfood for the sake of consistency across OTel Go repositories. Because I cannot find a clear answer myself, I can simply create a follow-up PR once this one is merged. It should be easier for the reviewers to decide.