Ensure changes to repo level source-build files are code reviewed by source-build experts #3435
Comments
|
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
|
This is an example of a change we should be reviewing. These prebuilts should have been added to SBRP. |
|
@oleksandr-didyk - FYI, It looks like a few repos are missing from the tracking list. There are 26 repos that participate in source-build. |
The list contains 24 + I omitted SBRP and externals as changes to them would get reviewed by us anyways. With them its 26, or did I miss some? |
IMO we should still update the two source-build repos for reference purposes (referring to the comments at the top of the source-build files, not CODEOWNERS). Our repos are a reference for others to see how to implement source-build. |
Sure thing, will add them |
|
@MichaelSimons something I forgot to ask - if a team is added to Is it worth asking the repo maintainers for this for the 2 files we currently have? |
Contributes to dotnet/source-build#3435 Adds comments to source-build files asking for the inclusion of the source-build team in PRs that alter `SourceBuild*` files. Non-reviewed changes could potentially cause issues down the line, be it in the downstream repos or the product build (as has happened in the past, see dotnet/source-build#3435 (comment))
[Triage] - We feel like that we should try and get access if repo owners are willing as this is the best way to ensure source-build experts are included in code reviews. If there is resistance/pushback, then we will drop the request to change CODEOWNERS. |
|
FYI - It appears that you cannot grant access or mention teams cross organization boundaries. So repos like vstest that reside in the Microsoft organization can't assign source-build-internal as a CODEOWNER. |
In that case, I suppose we could use individual usernames instead? |
I have thought about it but it is a maintenance concern and opted not to for now. Let's see how effective the comment is. If it remains a problem then we should use individuals. |
Context Contributes to dotnet/source-build#3435 Adds comments to source-build files asking for the inclusion of the source-build team in PRs that alter SourceBuild* files. Non-reviewed changes could potentially cause issues down the line, be it in the downstream repos or the product build (as has happened in the past, see dotnet/source-build#3435 (comment)) Changes Made added comments to source-build files asking for the inclusion of the source-build team in PRs that alter SourceBuild* file.
Contributes to dotnet/source-build#3435 Adds comments to source-build files asking for the inclusion of the source-build team in PRs that alter `SourceBuild*` files. Non-reviewed changes could potentially cause issues down the line, be it in the downstream repos or the product build (as has happened in the past, see dotnet/source-build#3435 (comment)) Add ownership restrictions to files. --------- Co-authored-by: Juan Hoyos <[email protected]>
|
Closing as done |
Here is an example change that should have been reviewed by a source-build expert. The changes to the SourceBuildPrebuiltBaseline.xml caused prebuilts to be added and weren't caught until internal source-build CI.
Suggestions:
Repos with comments only:
Repos with both the comments and
CODEOWNERSentry:The text was updated successfully, but these errors were encountered: