Skip to content

remove internal/matchers #6777

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 22, 2025
Merged

remove internal/matchers #6777

merged 4 commits into from
May 22, 2025

Conversation

codeimmortal
Copy link
Contributor

@codeimmortal codeimmortal commented May 15, 2025 •
edited by pellared
Loading

Fixes #6523

Remove internal/matchers from code base and its code generation

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented May 15, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: codeimmortal / name: codeimmortal (73bb321)
  • ✅ login: dmathieu / name: Damien Mathieu (58a1813)
  • ✅ login: pellared / name: Robert Pająk (ae7a126, 358fba4)

@dmathieu dmathieu added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 15, 2025
@dmathieu
Copy link
Member

This closes #6523

MrAlias
MrAlias reviewed May 15, 2025
View reviewed changes
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

The actual shared code templates (internal/shared/matchers) need to be removed as well.

@dmathieu
Copy link
Member

If you remove everything in this PR (which is fine) then, you're also closing #6541

@codeimmortal
Copy link
Contributor Author

codeimmortal commented May 15, 2025 •
edited
Loading

Sure let me work on it, looks like this(internal/shared/matchers) has dependency with https://github.com/open-telemetry/opentelemetry-go/blob/main/sdk/internal/internaltest/harness.go#L19 and trace/utils_test.go

@codeimmortal
Copy link
Contributor Author

Removed (internal/shared/matchers) and places where it is used. removed harness.go also as it completely use matchers.

@dmathieu dmathieu self-requested a review May 19, 2025 07:07
@dmathieu
Copy link
Member

If you remove the templates, you need to remove all uses. CI is currently failing because of that.
I would split this into multiple PRs though. I think your original commit was good.

@MrAlias
Copy link
Contributor

MrAlias commented May 19, 2025

I would split this into multiple PRs though. I think your original commit was good.

Likely the best approach. Sorry for the confusion.

@codeimmortal
Copy link
Contributor Author

Will Update PR for that today.

@codeimmortal
Copy link
Contributor Author

Make it like original PR. Remove only internal/matchers

MrAlias
MrAlias approved these changes May 21, 2025
View reviewed changes
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution 🎉

@dmathieu dmathieu merged commit a571bc1 into open-telemetry:main May 22, 2025
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmathieu dmathieu dmathieu approved these changes

@pellared pellared pellared approved these changes

@MrAlias MrAlias MrAlias approved these changes

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

Assignees
No one assigned
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove internal/matchers
4 participants
@codeimmortal @dmathieu @MrAlias @pellared