add review comment to sb files #87003
Conversation
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsContributes to dotnet/source-build#3435 Adds comments to source-build files asking for the inclusion of the source-build team in PRs that alter
|
eng/SourceBuild.props
Outdated
| @@ -1,3 +1,5 @@ | |||
| <!-- Whenever altering this or other Source Build files, please include @dotnet/source-build-internal as a reviewer. --> | |||
There was a problem hiding this comment.
Instead of adding this comment, add lines with this alias for these files to
/~https://github.com/dotnet/runtime/blob/main/.github/CODEOWNERS
It will ensure that you are automatically included on PRs that modify these.
There was a problem hiding this comment.
Initially wanted to add both the comment and the CODEOWNERS entry, but for a team to be added as a code owner it needs to have write permissions to the repo and I wanted to verify first if this is something we would need. I should've mentioned that in the description though, my bad, forgot to edit it.
If you are OK with granting write permissions to dotnet/source-build-internal, I will add the CODEOWNERS entry.
There was a problem hiding this comment.
Hmm, I did not know about the write access requirements. I have looked around and we do not have prior art of giving repo write access to team aliases. The current code owner entries that point to feature team aliases (like @dotnet/jit-contrib) are not effective.
We have automation setup by @terrajobst that monitors who has write access to the repos. I am not sure how it is going to react to granting write access to feature team aliases. This is something that would be best discussed over email first. You can start email thread about this with .NET OSS Admins.
There was a problem hiding this comment.
Thank you, will do
There was a problem hiding this comment.
The e-mail thread unfortunately is taking a bit longer then expected.
Would it make sense from your perspective to merge the PR with the comments only and return to the subject of CODEOWNERS entry later? Thank you.
ghost
added
the
needs-author-action
ghost
removed
the
needs-author-action
eng/SourceBuildPrebuiltBaseline.xml
Outdated
| @@ -1,4 +1,6 @@ | |||
| <!-- Whenever altering this or other Source Build files, please include @dotnet/source-build-internal as a reviewer. --> | |||
There was a problem hiding this comment.
| <!-- Whenever altering this or other Source Build files, please include @dotnet/source-build-internal as a reviewer. --> | |
| <!-- Whenever altering this file, please include @dotnet/source-build-internal as a reviewer. --> |
"other Source Build files" is ambiguous. It is not clear what those files are from the comment. Is it just the SourceBuild.props file?
There was a problem hiding this comment.
True, thank you for pointing it out.
Currently there are two (SourceBuild.props and SourceBuildPreBuiltBaseline.xml), but there might be more in the future.
Probably changing it to Whenever altering this or other SourceBuild* files would be more useful
There was a problem hiding this comment.
Improved the comment a bit to better characterize which exact files we are referring to.
oleksandr-didyk
force-pushed
the
add-sb-review-comment
branch
from
27b92bf to
98a1659
Compare
ghost
locked as resolved and limited conversation to collaborators
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))