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

Remove the obsolete, prefix-based CODEOWNERS matcher #5431

Merged
merged 2 commits into from
Feb 23, 2023

Conversation

konrad-jamrozik
Copy link
Contributor

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

A follow-up to:

With this PR, now only the new, regex-based, wildcard-supporting CODEOWNERS matcher is present in the codebase.

This PR also:

This PR contributes to:

Merging prerequisite

This PR can be merged only after the following PRs are merged and relevant NuGet package published

@konrad-jamrozik konrad-jamrozik added the Central-EngSys This issue is owned by the Engineering System team. label Feb 13, 2023
@konrad-jamrozik konrad-jamrozik self-assigned this Feb 13, 2023
@konrad-jamrozik konrad-jamrozik requested review from weshaggard, benbp and a team February 13, 2023 22:10
@konrad-jamrozik konrad-jamrozik changed the title Remove the obsolete, prefix-based CODEOWNERS matcher Remove the obsolete, prefix-based CODEOWNERS matcher & related tests Feb 13, 2023
@azure-sdk
Copy link
Collaborator

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

@konrad-jamrozik konrad-jamrozik marked this pull request as ready for review February 13, 2023 23:51
@konrad-jamrozik konrad-jamrozik changed the title Remove the obsolete, prefix-based CODEOWNERS matcher & related tests Remove the obsolete, prefix-based CODEOWNERS matcher Feb 14, 2023
@konrad-jamrozik konrad-jamrozik marked this pull request as draft February 14, 2023 06:52
ghost pushed a commit that referenced this pull request Feb 14, 2023
…le + remove obsolete tests (#5437)

This is a follow-up PR to:
- #5241

That PR was supposed to enable the regex-based matcher everywhere, but didn't. This PR fixes that.

This PR also:
- removes the obsolete `ProgramSimplePathTests` test class; it is superseded by `ProgramGlobPathTests`, which is renamed in:
    - #5431
- removes obsolete tests from `get-codeowners.ps1`

This PR is a prerequisite for following PRs:
- #5103
- #5431
ghost pushed a commit that referenced this pull request Feb 16, 2023
…ers` executable + add some tests; make assorted refactorings (#5103)

This PR is part of work required in preparation to merge:
- #5088

Specifically, to enable review of ownership changes due to upcoming wildcards support, as explained in:
- #5088 (comment)

As such, this PR contributes to:
- #2770

This PR is a prerequisite for following PRs:
- #5431

### Merging prerequisite

This PR can be merged only after the following PRs are merged and relevant NuGet package published
- #5437
- #5104

This is because this PR depends on that NuGet package.
remove ProgramSimplePathTests; rename ProgramGlobPathTests to RetrieveCodeOwnersProgramTests
remove the obsolete prefix-based `CODEOWNERS` matcher
@azure-sdk
Copy link
Collaborator

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

@@ -38,19 +36,17 @@ public static List<CodeownersEntry> GetCodeownersEntries(string codeownersConten

public static CodeownersEntry GetMatchingCodeownersEntry(
string targetPath,
string codeownersFilePathOrUrl,
bool useRegexMatcher = UseRegexMatcherDefault)
string codeownersFilePathOrUrl)
Copy link
Member

Choose a reason for hiding this comment

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

@JimSuplizio FYI if you are passing this third parameter and not using the default this will break when you get this update.

konrad-jamrozik pushed a commit to Azure/azure-sdk-for-js that referenced this pull request Feb 23, 2023
Sync eng/common directory with azure-sdk-tools for PR
Azure/azure-sdk-tools#5431 See [eng/common
workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow)

Co-authored-by: Konrad Jamrozik <kojamroz@microsoft.com>
@konrad-jamrozik
Copy link
Contributor Author

/check-enforcer override

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
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants