[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
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.)