-
Notifications
You must be signed in to change notification settings - Fork 23
feat: support non-root build paths #209
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
feat: support non-root build paths #209
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
==========================================
+ Coverage 51.22% 53.32% +2.10%
==========================================
Files 48 48
Lines 3274 3306 +32
==========================================
+ Hits 1677 1763 +86
+ Misses 1477 1420 -57
- Partials 120 123 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4a58638 to
22e237d
Compare
|
Not sure that we want to have code coverage as required. I usually don't like to require that, even less if right now great part of our app is tested via integration/e2e tests. |
63cdc08 to
3bfc313
Compare
|
@y1yang0 Could you have another look at this? |
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.
Pull request overview
This pull request adds support for building non-root package paths (e.g., otel go build -a ./app/vmctl) by implementing package discovery functionality. The implementation uses the golang.org/x/tools/go/packages library to correctly identify and process build targets from command-line arguments.
Key Changes
- Package Discovery: New
GetBuildPackagesfunction parsesgo buildarguments to identify target packages, handling flags, multiple packages, and wildcard patterns - Multi-package Support: The setup phase now generates
otel.runtime.gofiles in each target package directory rather than only in the root - Deterministic Imports: Import declarations in generated files are now sorted for consistent output
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tool/util/go.go | Adds GetBuildPackages function to extract package patterns from build arguments and load corresponding packages |
| tool/util/go_test.go | Comprehensive tests for package discovery covering single/multiple packages, wildcards, flags, and edge cases |
| tool/internal/setup/setup.go | Integrates package discovery to generate instrumentation files in each target package directory and update cleanup logic |
| tool/internal/setup/add.go | Updates addDeps to accept package path parameter, adds deterministic import ordering, and improves logging |
| tool/internal/setup/add_test.go | Golden file tests for the addDeps function covering various rule configurations |
| tool/internal/setup/testdata/*.golden | Golden files for testing generated otel.runtime.go content |
| go.mod | Moves golang.org/x/tools from indirect to direct dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
688e5dc to
08c44c3
Compare
|
I'll take a look tomorrow. Thanks |
y1yang0
left a comment
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.
Otherwise LGTM
143ee4f to
37206ef
Compare
37206ef to
458fb2f
Compare
458fb2f to
00d7c0d
Compare
Contributing to #196
Support building non-root paths e.g.
otel go build -a ./app/vmctl