Skip to content

Cleanup resulting from enabling nullables. - #372

Merged
simeoncran merged 2 commits into
masterfrom
NullablesCleanup
Oct 20, 2020
Merged

simeoncran merged 2 commits into
masterfrom
NullablesCleanup

Conversation

@simeoncran

Copy link
Copy Markdown
Contributor

Replace Asserts that are now handled by nullable analysis.
Add some more "#nullable enable" to quieten code analysis in VS.
Rename a method in CodeBuilder because it referred to Cpp but is now used by C# as well.

Replace Asserts that are now handled by nullable analysis.
Add some more "#nullable enable" to quieten code analysis in VS.
Rename a method in CodeBuilder because it referred to Cpp but is now used by C# as well.
@simeoncran
simeoncran requested a review from a team as a code owner October 20, 2020 17:16

static void TransferShapeProperties(CompositionShape from, CompositionShape to)
{
void TransferProperty<T>(Func<CompositionShape, T> get, Action<CompositionShape, T> set)

@derobinsMSFT derobinsMSFT Oct 20, 2020 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TransferProperty [](start = 17, length = 16)

Curious about the changes in this file-- why was this method separated? The new methods appear identical other than what type T is allowed to be. #Resolved

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's to allow T? into the type signature. As soon as you have a T?, you have to differentiate between value types and reference types, otherwise it's not clear to the compiler what the ? means. So I add the ?, and added a constraints for the class and struct versions.
You can't overload methods based on constraints, so it also required giving the methods distinct names.


In reply to: 508724228 [](ancestors = 508724228)

@derobinsMSFT derobinsMSFT left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:shipit:

@simeoncran
simeoncran merged commit 09224fc into master Oct 20, 2020
@delete-merged-branch
delete-merged-branch Bot deleted the NullablesCleanup branch October 20, 2020 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants