Skip to content
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

Fix suffix * handling in our CODEOWNERS path matcher #5308

Merged

Conversation

konrad-jamrozik
Copy link
Contributor

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

As in subject. Prompted by this comment thread:

There were some other issues with the matcher being inconsistent with GitHub's matcher. For details, please see this comment:

This PR contributes to:

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Feb 1, 2023
@konrad-jamrozik konrad-jamrozik requested a review from a team as a code owner February 1, 2023 21:54
@konrad-jamrozik konrad-jamrozik self-assigned this Feb 1, 2023
@konrad-jamrozik konrad-jamrozik changed the title Fix suffix single start handling in our CODEOWNERS path matcher Fix suffix * handling in our CODEOWNERS path matcher Feb 1, 2023
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/fix_suffix_single_star_handling branch from 636a63c to 94f785a Compare February 1, 2023 21:56
@konrad-jamrozik
Copy link
Contributor Author

konrad-jamrozik commented Feb 2, 2023

@weshaggard I made some significant changes since you approved; turns out, GitHub's CODEOWNERS matching logic is a bit inconsistent when it comes to handling suffixes. I did manual tests using my own scratchpad repo to confirm its behavior. The behavior is explained here.

In a nutshell:

  • For top level dir, / doesn't work. One has to use /**.
  • But for nested dirs, / works. I.e. one can write /foo/ and it is equivalent to /foo/**.
  • * has different interpretations. If used with preceding slash, like /* or /foo/*
    it means "things only in this dir". But when used as a suffix, it means "anything".
    So /foo* is effectively /foo*/** OR /foo*. Where * in the OR clause
    should be interpreted as "any character except /".

@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/fix_suffix_single_star_handling branch 3 times, most recently from 19fc6e0 to 3a38250 Compare February 2, 2023 06:49
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/fix_suffix_single_star_handling branch from 3a38250 to 446a087 Compare February 2, 2023 06:50
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