Skip to content

Fix regex-based matcher to be a strict subset of GitHub-based CODEOWNERS matcher.#5330

Merged
konrad-jamrozik merged 3 commits intomainfrom
users/kojamroz/matcher_nostar
Feb 8, 2023
Merged

Fix regex-based matcher to be a strict subset of GitHub-based CODEOWNERS matcher.#5330
konrad-jamrozik merged 3 commits intomainfrom
users/kojamroz/matcher_nostar

Conversation

@konrad-jamrozik
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik commented Feb 3, 2023

As in subject. This PR is a follow-up to:

This PR contributes to:

Most notably, now the matcher is a strict subset of GitHub CODEOWNERS matcher. That is, for each CODEOWNERS path our matcher should behave exactly the same as GitHub CODEOWNERS path matcher, except when that path violates additional validation imposed by us (see the MatchedCodeownersEntry class comments for details). In such case, given CODEOWNERS path is skipped (ignored) by our matcher, never matching any path.

Note that to make the matcher be a subset of the GitHub matcher, it can no longer normalize paths missing prefix / by adding it. This change was made in this PR.

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Feb 3, 2023
@konrad-jamrozik konrad-jamrozik requested a review from a team as a code owner February 3, 2023 03:20
@konrad-jamrozik konrad-jamrozik self-assigned this Feb 3, 2023
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/matcher_nostar branch 2 times, most recently from c646f46 to 11b457c Compare February 3, 2023 03:23
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/matcher_nostar branch from 7e6e1f2 to a64f734 Compare February 3, 2023 23:49
@konrad-jamrozik konrad-jamrozik changed the title Fix regex-based matcher to align with GitHub-based CODEOWNERS matcher when the path doesn't end with / nor *. Fix regex-based matcher to be a strict subset of GitHub-based CODEOWNERS matcher. Feb 4, 2023
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/matcher_nostar branch from 4632a24 to c2d4947 Compare February 4, 2023 04:00
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/matcher_nostar branch from c2d4947 to dde6923 Compare February 4, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Central-EngSys This issue is owned by the Engineering System team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments