Skip to content
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

[chore] make checkapi a module, adding a metadata.yaml #37945

Merged
merged 6 commits into from
Mar 5, 2025

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Feb 14, 2025

Description

This PR makes the checkapi tool a go module with a metadata.yaml.

@atoulme atoulme requested a review from a team as a code owner February 14, 2025 22:54
@atoulme atoulme requested a review from dehaansa February 14, 2025 22:54
@atoulme atoulme force-pushed the make_checkapi_a_module branch from 91bd7a1 to 2793fcf Compare February 14, 2025 23:13
Copy link
Contributor

@dehaansa dehaansa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing

go: warning: "./..." matched no packages
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
       find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
find: illegal option -- n
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
       find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]
no Go files in /Users/dehaansa/go/src/github.com/open-telemetry/opentelemetry-collector-contrib

Whenever I run a make task on this branch. Rescinding approval while I take a look.

Edit: Nothing discovered that makes any sense here. Happy to treat this as a "my machine" issue if someone else approves. On macOS sequoia, this behavior doesn't happen for me on other branches, I pulled a few other PRs to check.

@atoulme
Copy link
Contributor Author

atoulme commented Feb 15, 2025

I see it too. It's to do with how the checkapi binary is built, probably within the wrong working directory. I will look into this more.

@atoulme
Copy link
Contributor Author

atoulme commented Feb 15, 2025

Let's get #37951 in first, this PR has brought in unexpected issues.

MovieStoreGuy pushed a commit that referenced this pull request Feb 19, 2025
…up (#37951)

This change is brought about by the realization that with #37945 no code
is directly under the root module. This has the unexpected impact that
some of the Makefile set up started to show warnings.

It turns out that we do quite a bit of reading files and go packages in
the Makefile. This has a performance impact. We also have incorrect code
in there, such as the computation of `ALL_PKG_DIRS` only returns
`github.com/open-telemetry/opentelemetry-collector-contrib/cmd/checkapi`.
@atoulme atoulme force-pushed the make_checkapi_a_module branch from 5dd9687 to cea322f Compare February 20, 2025 00:19
@atoulme atoulme requested a review from dehaansa February 20, 2025 00:20
@atoulme
Copy link
Contributor Author

atoulme commented Feb 23, 2025

@dehaansa ready for another look

@atoulme atoulme force-pushed the make_checkapi_a_module branch 2 times, most recently from 94a20df to 8d6b37a Compare March 4, 2025 16:45
@atoulme atoulme force-pushed the make_checkapi_a_module branch from 8d6b37a to 7105a89 Compare March 4, 2025 19:28
Co-authored-by: Sam DeHaan <dehaansa@gmail.com>
@atoulme atoulme dismissed dehaansa’s stale review March 5, 2025 01:11

stale review

@atoulme atoulme merged commit 251f677 into open-telemetry:main Mar 5, 2025
155 of 156 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants