Fix globstar expansion in CI Vis code owners - #9045
Draft
bouwkast wants to merge 6 commits into
Draft
Conversation
andrewlock
reviewed
Aug 13, 2026
Comment on lines
+144
to
+147
| rx = rx.Replace("/\\*\\*/", "/(?:.*/)?"); | ||
|
|
||
| // A leading globstar followed by a slash also matches at the repository root. | ||
| rx = rx.Replace("\\*\\*/", "(?:.*/)?"); |
Member
There was a problem hiding this comment.
I don't think we want . because that matches / as well
Suggested change
| rx = rx.Replace("/\\*\\*/", "/(?:.*/)?"); | |
| // A leading globstar followed by a slash also matches at the repository root. | |
| rx = rx.Replace("\\*\\*/", "(?:.*/)?"); | |
| rx = rx.Replace("/\\*\\*/", "/(?:[^/]*/)?"); | |
| // A leading globstar followed by a slash also matches at the repository root. | |
| rx = rx.Replace("\\*\\*/", "(?:[^/]*/)?"); |
bouwkast
marked this pull request as draft
August 13, 2026 16:04
Collaborator
Author
|
Will re-raise this next week-ish |
BenchmarksBenchmark execution time: 2026-08-13 16:35:09 Comparing candidate commit 92d59f0 in PR branch Found 1 performance improvements and 1 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 66 known flaky benchmarks, 60 flaky benchmarks without significant changes.
|
andrewlock
added a commit
that referenced
this pull request
Aug 14, 2026
## Summary of changes Tweak codeowners to work around a bug in our testlogger ## Reason for change Currently there's a bug in the CODEOWNERS parser with the pattern `something/**/here` which means it recognizes `/something/else/here` but not `/something/else`. This is a workaround, until we fix the test logger ## Implementation details Add the extra entry ## Test coverage errr ## Other details See #9045 for the real fix #incident-57303
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
2 participants
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
Makes it so that
/**/patterns when expanded/computed/regexed by CI Visibility will handle cases where there are zero matches.Reason for change
We ran into this issue with
/tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/**/DataStreamsMonitoring*in our CODEOWNERs where this specific testDatadog.Trace.ClrProfiler.IntegrationTests.DataStreamsMonitoringHttpClientTestsat the top level ofDatadog.Trace.ClrProfiler.IntegrationTestswasn't being matched to the correct teams in CI Visibility.This was because
/**/was being interpreted as/.*/for the cases with no directory (works fine for cases where there are directories though)/.*/-> match 0 to N characters but if no matches becomes//exactly.Implementation details
This changes
/.*/to/(?:.*/)?Test coverage
some basic tests added
Also played around with it in regex101 to see if it works and it seems like it does
Other details