-
Notifications
You must be signed in to change notification settings - Fork 493
chore: identify candidate dependencies for removal/replacement/absorption #4176
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
base: main
Are you sure you want to change the base?
Conversation
BenchmarksBenchmark execution time: 2025-11-28 09:54:16 Comparing candidate commit 183054e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 0 unstable metrics. |
kakkoyun
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.
Thanks for identifying all these. Extremely helpful.
My question is do we need to actually merge this PR?
We can keep this open as draft, or add another markdown document with check list until we tackle all.
I'm just not sure if it is a good idea to have these in the code comments.
|
@kakkoyun Agreed. There is no real need to merge them, although it's nice to have this in an easy format to review. I can set it to draft and create PRs linking to this one. |
eliottness
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.
wondering if they are function we use from golang.com/x/<...> that have made it into the standard library in the meantime
I can check them. I didn't prioritize them because, even if they are |
| github.com/open-feature/go-sdk v1.17.0 | ||
| github.com/puzpuzpuz/xsync/v3 v3.5.1 | ||
| // Could this one be isolated? | ||
| github.com/quasilyte/go-ruleguard/dsl v0.3.22 |
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.
@kakkoyun Is this dependency needed in the root module?
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.
It is only needed for the CI check and it's under a build flag. We can experiment how we can remove it from the root module. It's definitely not needed for the runtime, it's a build time dependency.
|
I checked the |
What does this PR do?
Identifies all the dependencies that might be removed, replaced by an already present similar dependency or absorbed into
dd-trace-go.Motivation
Reduce the number of dependencies to reinforce our supply chain.
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!