[chore] make generate uses .tools/mdatagen
#36416
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jade-guiton-dd
changed the title
make generate uses .tools/mdatagen and respects GROUPmake generate uses .tools/mdatagen
|
It seems marking the PR as ready for review, then turning into a draft again temporarily caused two people to be assigned, sorry about that. |
mx-psi
approved these changes
Nov 19, 2024
bogdandrutu
approved these changes
Nov 19, 2024
sbylica-splunk
pushed a commit
to sbylica-splunk/opentelemetry-collector-contrib
that referenced
this pull request
Dec 17, 2024
#### Description This PR is in the context of [this PR on the core Collector modifying `make check-contrib` to use the latest version of `mdatagen`](open-telemetry/opentelemetry-collector#11670). The `make generate` commands currently starts by compiling build tools in the `.tools` folder, then it `go install`s mdatagen globally for use in `go:generate`. Unfortunately, `go install` does not take into account the version of `mdatagen` referenced in `internal/tools/go.mod`. This means it isn't possible to generate using the local version of `mdatagen` for testing, even with `replace` statements. This PR fixes this by prepending `.tools` to the `$PATH` before running the generate command, which makes `go:generate` use the version of `mdatagen` in that folder instead of a global one. (Originally, this PR also added a new `modgenerate` target in `Makefile.Common`, to allow running generate commands on a specific group of modules instead of all of them for efficiency reasons. After discussing it with @mx-psi, we decided to hold off on that change for a later PR, as it warrants further discussion.)
AkhigbeEromo
pushed a commit
to sematext/opentelemetry-collector-contrib
that referenced
this pull request
Jan 13, 2025
#### Description This PR is in the context of [this PR on the core Collector modifying `make check-contrib` to use the latest version of `mdatagen`](open-telemetry/opentelemetry-collector#11670). The `make generate` commands currently starts by compiling build tools in the `.tools` folder, then it `go install`s mdatagen globally for use in `go:generate`. Unfortunately, `go install` does not take into account the version of `mdatagen` referenced in `internal/tools/go.mod`. This means it isn't possible to generate using the local version of `mdatagen` for testing, even with `replace` statements. This PR fixes this by prepending `.tools` to the `$PATH` before running the generate command, which makes `go:generate` use the version of `mdatagen` in that folder instead of a global one. (Originally, this PR also added a new `modgenerate` target in `Makefile.Common`, to allow running generate commands on a specific group of modules instead of all of them for efficiency reasons. After discussing it with @mx-psi, we decided to hold off on that change for a later PR, as it warrants further discussion.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is in the context of this PR on the core Collector modifying
make check-contribto use the latest version ofmdatagen.The
make generatecommands currently starts by compiling build tools in the.toolsfolder, then itgo installs mdatagen globally for use ingo:generate. Unfortunately,go installdoes not take into account the version ofmdatagenreferenced ininternal/tools/go.mod. This means it isn't possible to generate using the local version ofmdatagenfor testing, even withreplacestatements.This PR fixes this by prepending
.toolsto the$PATHbefore running the generate command, which makesgo:generateuse the version ofmdatagenin that folder instead of a global one.(Originally, this PR also added a new
modgeneratetarget inMakefile.Common, to allow running generate commands on a specific group of modules instead of all of them for efficiency reasons. After discussing it with @mx-psi, we decided to hold off on that change for a later PR, as it warrants further discussion.)