Skip to content

Rename IsNotNullableIfReferenceType to IsNotNullable #36708

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.

Sign up for GitHub

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

Jump to bottom
Merged
AlekseyTs merged 1 commit into from
Jun 25, 2019
Merged

Rename IsNotNullableIfReferenceType to IsNotNullable #36708

AlekseyTs merged 1 commit into from
Jun 25, 2019

Conversation

@AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented Jun 24, 2019 •
edited
Loading

All the affected APIs are also covering nullable value types now.

@AlekseyTs AlekseyTs requested a review from a team as a code owner June 24, 2019 18:30
@gafter
Copy link
Member

gafter commented Jun 24, 2019

Can you please provide some discussion about the purpose and motivation for this change?

jcouv
jcouv reviewed Jun 24, 2019
View reviewed changes
src/Compilers/CSharp/Portable/Symbols/TypeParameterSymbol.cs

// https://github.com/dotnet/roslyn/issues/26198 Should this API be exposed through ITypeParameterSymbol?
internal abstract bool? IsNotNullableIfReferenceType { get; }
internal abstract bool? IsNotNullable { get; }
Copy link
Member

@jcouv jcouv Jun 24, 2019

Choose a reason for hiding this comment

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

IsNotNullable [](start = 32, length = 13)

nit: Consider adding an xml doc while you're here

jcouv
jcouv approved these changes Jun 24, 2019
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 1)

@jcouv jcouv self-assigned this Jun 24, 2019
@jcouv jcouv added this to the 16.2.P4 milestone Jun 24, 2019
@AlekseyTs
Copy link
Contributor Author

AlekseyTs commented Jun 24, 2019

Can you please provide some discussion about the purpose and motivation for this change?

Done.

@AlekseyTs
Copy link
Contributor Author

AlekseyTs commented Jun 24, 2019

@dotnet/roslyn-compiler Please review, need a second sign-off for a pure rename change.

@AlekseyTs
Copy link
Contributor Author

AlekseyTs commented Jun 24, 2019 •
edited
Loading

@dotnet/roslyn-infrastructure It looks like integration tests are still flaky (https://github.com/dotnet/roslyn/pull/36708/checks?check_run_id=154757957, https://dev.azure.com/dnceng/public/_build/results?buildId=236648)

@jasonmalinowski
Copy link
Member

jasonmalinowski commented Jun 24, 2019

@JoeRobich @ivanbasov Can you take a look at @AlekseyTs's failure there? Only one of the four integration test legs failed, so it indeed looks like some flakiness the automatic rerun didn't pick up.

@AlekseyTs AlekseyTs merged commit 1bf4f5f into dotnet:master Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcouv jcouv jcouv approved these changes

+2 more reviewers

@gafter gafter gafter approved these changes

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jcouv jcouv

Labels

Area-Compilers Feature - Nullable Reference Types Nullable Reference Types

Projects

None yet

Milestone

16.2.P4

Development

Successfully merging this pull request may close these issues.

5 participants

@AlekseyTs @gafter @jasonmalinowski @cston @jcouv