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

Replace the glob-based CODEOWNERS parser with regex-based #5063

Merged
merged 9 commits into from
Jan 9, 2023

Conversation

konrad-jamrozik
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik commented Jan 5, 2023

This PR is a follow-up to:

It addresses:

It rewrites the new parser logic to match the rules as outlined in:

The new parser now supports diagnostic logging via Microsoft.Extensions.Logging.Console. To make this work, I took a dependency on this package, version 7.0.0, and updated that package and Microsoft.Extensions.Logging to 7.0.0 in the code that uses CodeOwnersParser as dependency, to avoid the "package version downgrade detected" build failure.

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Jan 5, 2023
@konrad-jamrozik konrad-jamrozik self-assigned this Jan 5, 2023
@konrad-jamrozik konrad-jamrozik requested a review from a team as a code owner January 5, 2023 16:09
@konrad-jamrozik konrad-jamrozik changed the title Add regex-based support for wildcards in CODEOWNERS file parser (disabled, behind a feature flag) Replace the glob-based CODEOWNERS parser with regex-based. Jan 5, 2023
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from ebfff5f to 9e188b1 Compare January 5, 2023 16:10
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from 9e188b1 to cdfb8b1 Compare January 5, 2023 22:25
@konrad-jamrozik konrad-jamrozik changed the title Replace the glob-based CODEOWNERS parser with regex-based. Replace the glob-based CODEOWNERS parser with regex-based Jan 6, 2023
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for removing the PackageReference I think we can move my other questions to your main PR. So after removing that reference go ahead and merge this.

Base automatically changed from users/kojamroz/iss_2770_tests to main January 7, 2023 05:01
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from cdfb8b1 to 5102e0e Compare January 7, 2023 05:04
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from 5aaa156 to 5d55bb5 Compare January 7, 2023 14:30
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch 2 times, most recently from 225892d to 62f1269 Compare January 7, 2023 15:14
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from 62f1269 to ee2a4e1 Compare January 7, 2023 15:15
@konrad-jamrozik konrad-jamrozik force-pushed the users/kojamroz/iss_2770_tests2 branch from 3a4fe53 to 98dbf7d Compare January 9, 2023 03:46
ghost pushed a commit that referenced this pull request Jan 14, 2023
…r all paths matching given glob path. (#5134)

This PR implements for the `retrieve-codeowners` tool the ability to return not only owners for a single path, but a list of all owners for all paths resolved when matching a glob path given as input.

As such, this PR contributes to:
- #5135

This work is necessary to be able to compare and diff the owners of all files in repository before and after the regex matcher is turned on, per this comment:
- #5088 (review)

In other words, this PR contributes to unblocking to the following PR:
- #5088

And as such, also contributes to:
- #2770

This PR will require some upstream changes, which are captured by:
- #5103

### Additional changes

- Removed logger from `CodeOwnersParser`; replaced with `Console.Out` and `Console.Error`. This addresses the following comment:
    - #5063 (comment)
- Prevented the new regex matcher from matching paths that have `*` in them - such paths are malformed anyway (at least on Windows).
- A lot of assorted changes to surrounding production & test code - please see the file diff for details.

### Implementation notes

This PR leverages [`Microsoft.Extensions.FileSystemGlobbing`](https://www.nuget.org/packages/Microsoft.Extensions.FileSystemGlobbing).
Doc: https://learn.microsoft.com/en-us/dotnet/core/extensions/file-globbing
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