Fix missing owners for path error to include path #8450
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JimSuplizio
added
the
CODEOWNERS Linter
weshaggard
reviewed
Jun 14, 2024
tools/codeowners-utils/Azure.Sdk.Tools.CodeownersUtils/Constants/ErrorMessageConstants.cs
Outdated
Show resolved
Hide resolved
weshaggard
approved these changes
Jun 14, 2024
This was referenced Jun 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem right now is that the missing owners error,
There are no owners defined for CODEOWNERS entry., is too generic. At the moment, the only missing owners in any of the repositories running the linter are for source path/owner lines. The fix here is to add the path to the error in order to make it distinct.There is one big issue here:
Monikers can also have missing owners but because we can't put line numbers into the baseline file, for obvious reasons, there's no good differentiation for monikers missing errors.
The mitigation for the issue:
The only repositories that have this error in their baseline files are azure-sdk-for-java and azure-sdk-for-js. For Java, this error is actually because of two source path/owner lines lacking owners. For JS, this error was apparently fixed and never removed from the baseline file. When this change goes in, and the linter version is updated, Java's baseline file will need to be updated with the new errors, which will include the path. This means that any Moniker's missing owners will fail the pipeline at the PR level since these don't currently exist in any repository running the linter.
This change only affects the linter. The parser, which is used by a number of tools, remains unchanged. Only the linter version is going to need to be updated when this is merged.