Rework CODEOWNERS tests to try and aim for exclusive ownership - #9155
Draft
bouwkast wants to merge 12 commits into
Draft
Rework CODEOWNERS tests to try and aim for exclusive ownership#9155bouwkast wants to merge 12 commits into
bouwkast wants to merge 12 commits into
Conversation
Collaborator
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d3f23195
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # We need to update the test logger to fix this. | ||
| tracer/ @DataDog/apm-sdk-capabilities-dotnet @DataDog/apm-lang-platform-dotnet @DataDog/tracing-dotnet | ||
| /tracer/build/ @DataDog/apm-dotnet | ||
| /tracer/test/Datadog.Trace.TestHelpers/ @DataDog/apm-lang-platform-dotnet |
There was a problem hiding this comment.
When a PR changes a file under Datadog.Trace.TestHelpers, this last-matching rule removes @DataDog/tracing-dotnet, but CommonTracerChanges() only recognizes this explicitly listed shared directory after confirming that its matched owners contain TracingDotnet. Since the rule also lacks @DataDog/asm-dotnet, isAsmChanged remains false and the ASM test matrices are omitted, even though both Security test projects reference this helper project. Retain the tracing co-owner here or make the common-directory check independent of ownership.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Tried expanding this to be handled.
BenchmarksBenchmark execution time: 2026-09-01 14:49:48 Comparing candidate commit 5e25f9c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 0 unstable metrics, 66 known flaky benchmarks, 21 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9155) and master.
|
|||||||||||||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 201.13 ± (201.15 - 202.03) ms | 216.68 ± (216.38 - 217.55) ms | +7.7% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 206.79 ± (206.24 - 207.27) ms | 220.56 ± (220.27 - 221.25) ms | +6.7% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1206.04 ± (1205.34 - 1212.01) ms | 1286.23 ± (1284.16 - 1292.45) ms | +6.6% | ❌⬆️ |
Co-authored-by: Andrew Lock <andrewlock.net@gmail.com>
bouwkast
force-pushed
the
steven/codeowners-9
branch
from
August 31, 2026 15:45
d66e975 to
751017b
Compare
bouwkast
force-pushed
the
steven/codeowners-24-2
branch
from
August 31, 2026 15:48
a0d3f23 to
efb2a0e
Compare
github-actions
Bot
added
the
area:builds
![datadog-datadog-demo-org[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
bouwkast
force-pushed
the
steven/codeowners-9
branch
from
September 1, 2026 12:39
751017b to
1c89993
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary of changes
Further refines to ensure that every test is exclusively owned by an "owning" team and not a "reviewing" team.
Reason for change
Being asked to reframe teams that are "reviewing" teams such as "tracing-dotnet" to not be the exclusive owner of any test and instead ensure that every single test is owned by some product team.
Implementation details
I went through the
/tracer/testdirectory and files (with a nice and assigned a team, or teams, that I thought "best" owned the file.Additionally, there are some changes in the
tracer/build/_build/Build.VariableGenerations.csfile as we usetracing-dotnetto determine whether to run. Ideally we have a better solution, but we want/need this change sooner rather than later.tracing-dotnetcan / will be added as a "reviewer" team where needed (it is just a co-owner). We have some internal tooling/reporting to differentiate this team as "not an owner"Test coverage
Other details
Some of these are probably wrong, we should treat this as a iterative improvement / process, but we must ensure that tracing-dotnet / apm-dotnet are not the exclusive owner of any test sooner rather than later.