Add to the `retrieve-codeowners` tool support for returning owners for all paths matching given glob path. by konrad-jamrozik · Pull Request #5134 · Azure/azure-sdk-tools

konrad-jamrozik · 2023-01-13T09:48:07Z

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:

Implement tool for retrieving owners of all repository files. #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:

Enable the new, regex-based, wildcard-supporting CODEOWNERS matcher #5088 (review)

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

Enable the new, regex-based, wildcard-supporting CODEOWNERS matcher #5088

And as such, also contributes to:

Add support for wildcard handling of file matching in codeowners library #2770

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

Update get-codeowners.ps1 to work with the updated RetrieveCodeOwners executable + add some tests; make assorted refactorings #5103

Additional changes

Removed logger from CodeOwnersParser; replaced with Console.Out and Console.Error. This addresses the following comment:
- Replace the glob-based CODEOWNERS parser with regex-based #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.
Doc: https://learn.microsoft.com/en-us/dotnet/core/extensions/file-globbing

tools/code-owners-parser/CodeOwnersParser/CodeownersEntry.cs

tools/code-owners-parser/CodeOwnersParser/Azure.Sdk.Tools.CodeOwnersParser.csproj

azure-sdk · 2023-01-13T10:08:51Z

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

azure-sdk · 2023-01-13T10:16:47Z

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

weshaggard · 2023-01-13T21:01:03Z

...ser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/Azure.Sdk.Tools.RetrieveCodeOwners.Tests.csproj


  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
+    <LangVersion>10.0</LangVersion>


Is this necessary to specify? What is the default language feature?

@weshaggard Prompted by your question I investigated a bit and I think it is coming from Directory.Build.props. I will make a PR to update this to 10.0, now that we have moved everything to net6.0.

Submitted PR:

Unhardcode C# lang versions, making them use TFM default (currently 10.0 due to TFM of net6.0). #5175

...e-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Azure.Sdk.Tools.RetrieveCodeOwners.csproj

tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs

tools/code-owners-parser/CodeOwnersParser/GlobFilePath.cs

tools/code-owners-parser/CodeOwnersParser/MatchedCodeownersEntry.cs

weshaggard

A few questions/comments but otherwise seems reasonable.

azure-sdk · 2023-01-14T00:35:41Z

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

ghost · 2023-01-14T01:12:26Z

Hello @azure-sdk!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Jan 13, 2023

konrad-jamrozik requested review from sima-zhu and weshaggard January 13, 2023 09:48

konrad-jamrozik self-assigned this Jan 13, 2023

konrad-jamrozik requested a review from a team as a code owner January 13, 2023 09:48

konrad-jamrozik commented Jan 13, 2023

View reviewed changes

tools/code-owners-parser/CodeOwnersParser/CodeownersEntry.cs Show resolved Hide resolved

konrad-jamrozik commented Jan 13, 2023

View reviewed changes

tools/code-owners-parser/CodeOwnersParser/Azure.Sdk.Tools.CodeOwnersParser.csproj Show resolved Hide resolved

konrad-jamrozik mentioned this pull request Jan 13, 2023

Implement tool for retrieving owners of all repository files. #5135

Closed

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from f88fb89 to 5659e9e Compare January 13, 2023 09:59

konrad-jamrozik requested a review from benbp as a code owner January 13, 2023 09:59

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from 5659e9e to 95c3691 Compare January 13, 2023 09:59

konrad-jamrozik changed the title ~~Add to retrieve-codeowners tools support for returning owners for all paths matching given glob path.~~ Add to the retrieve-codeowners tool support for returning owners for all paths matching given glob path. Jan 13, 2023

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from 95c3691 to f7a5ade Compare January 13, 2023 10:09

ongoing

762bdf4

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from f7a5ade to 762bdf4 Compare January 13, 2023 10:10

konrad-jamrozik mentioned this pull request Jan 13, 2023

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

Merged

weshaggard reviewed Jan 13, 2023

View reviewed changes

...e-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Azure.Sdk.Tools.RetrieveCodeOwners.csproj Show resolved Hide resolved

weshaggard reviewed Jan 13, 2023

View reviewed changes

tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs Outdated Show resolved Hide resolved

weshaggard reviewed Jan 13, 2023

View reviewed changes

tools/code-owners-parser/CodeOwnersParser/GlobFilePath.cs Show resolved Hide resolved

weshaggard reviewed Jan 13, 2023

View reviewed changes

tools/code-owners-parser/CodeOwnersParser/MatchedCodeownersEntry.cs Show resolved Hide resolved

weshaggard approved these changes Jan 13, 2023

View reviewed changes

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from 2eb8b7e to b1d1774 Compare January 14, 2023 00:28

address PR remarks

3ed5c92

konrad-jamrozik force-pushed the users/kojamroz/tool_add_owners_diff branch from b1d1774 to 3ed5c92 Compare January 14, 2023 00:28

This was referenced Jan 14, 2023

Sync eng/common directory with azure-sdk-tools for PR 5134 Azure/azure-sdk-for-android#1283

Merged

Sync eng/common directory with azure-sdk-tools for PR 5134 Azure/azure-sdk-for-c#2478

Merged

azure-sdk added the auto-merge label Jan 14, 2023

ghost merged commit 4cf0beb into main Jan 14, 2023

ghost deleted the users/kojamroz/tool_add_owners_diff branch January 14, 2023 01:14

This was referenced Jan 19, 2023

Implement tools for diffing sets of owners of repo paths; update the regex-based CODEOWNERS matching logic #5165

Merged

Unhardcode C# lang versions, making them use TFM default (currently 10.0 due to TFM of net6.0). #5175

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add to the `retrieve-codeowners` tool support for returning owners for all paths matching given glob path.#5134

Add to the `retrieve-codeowners` tool support for returning owners for all paths matching given glob path.#5134
2 commits merged intomainfrom
users/kojamroz/tool_add_owners_diff

konrad-jamrozik commented Jan 13, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

azure-sdk commented Jan 13, 2023

Uh oh!

azure-sdk commented Jan 13, 2023

Uh oh!

weshaggard Jan 13, 2023

Uh oh!

konrad-jamrozik Jan 14, 2023 •

edited

Loading

Uh oh!

konrad-jamrozik Jan 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

weshaggard left a comment

Uh oh!

azure-sdk commented Jan 14, 2023

Uh oh!

ghost commented Jan 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

konrad-jamrozik commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional changes

Implementation notes

Uh oh!

Uh oh!

Uh oh!

azure-sdk commented Jan 13, 2023

Uh oh!

azure-sdk commented Jan 13, 2023

Uh oh!

weshaggard Jan 13, 2023

Choose a reason for hiding this comment

Uh oh!

konrad-jamrozik Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

konrad-jamrozik Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

weshaggard left a comment

Choose a reason for hiding this comment

Uh oh!

azure-sdk commented Jan 14, 2023

Uh oh!

ghost commented Jan 14, 2023

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

konrad-jamrozik commented Jan 13, 2023 •

edited

Loading

konrad-jamrozik Jan 14, 2023 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.