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

add review comment to sb files #15288

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

oleksandr-didyk
Copy link
Contributor

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))

@vzarytovskii
Copy link
Member

vzarytovskii commented Jun 1, 2023

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))

Probably just add those to codeowners file? It will automatically request the review:

https://github.com/dotnet/fsharp/blob/main/.github/CODEOWNERS

@oleksandr-didyk
Copy link
Contributor Author

oleksandr-didyk commented Jun 2, 2023

Probably just add those to codeowners file?

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.

@vzarytovskii
Copy link
Member

dotnet/source-build-internal

Added the team

@oleksandr-didyk
Copy link
Contributor Author

Added the team

Thank you, added an entry for source-build files into CODEOWNERS

@KevinRansom KevinRansom merged commit 7fd0aa5 into dotnet:main Jun 2, 2023
T-Gro pushed a commit that referenced this pull request Jun 5, 2023
* add review comment to sb files

* add CODEOWNERS entry for source-build

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
T-Gro pushed a commit that referenced this pull request Jun 5, 2023
* add review comment to sb files

* add CODEOWNERS entry for source-build

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
psfinaki pushed a commit that referenced this pull request Jun 6, 2023
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Sudqi <sudqiomar@gmail.com>
Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: kerams <kerams@users.noreply.github.com>
T-Gro added a commit that referenced this pull request Jun 6, 2023
* add review comment to sb files (#15288)

* add review comment to sb files

* add CODEOWNERS entry for source-build

* Don't show inline hint for arguments with same names as the parameters in DU (#15305)

* Signature of nested type with generic type parameter (#15259)

* Proof of concept

* Add generic parameter names to ModuleOrType.

* Revert ModuleOrType change

* Process ticks in demangledPath of TType_app.

* Only apply new logic when includeStaticParametersInTypeNames is active.

* Use FactForNETCOREAPP

* Fix build

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Improve implied lambda and delegate argument names (#15277)

* Improve implied lambda and delegate argument names

* Fix

* Add tests

* Revert non-preview tests

* Sigh

* Re-revert

* Fix test

* Add testx

---------

Co-authored-by: Oleksandr Didyk <106967057+oleksandr-didyk@users.noreply.github.com>
Co-authored-by: Sudqi <sudqiomar@gmail.com>
Co-authored-by: Florian Verdonck <florian.verdonck@outlook.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: kerams <kerams@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants