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

[WIP] C# 8 nullable reference type support #102

Closed
wants to merge 25 commits into from
Closed

[WIP] C# 8 nullable reference type support #102

wants to merge 25 commits into from

Conversation

sungam3r
Copy link
Member

@sungam3r sungam3r commented Oct 28, 2019

Rebased #99

I did not understand how to do rebase right, the branch was deleted and the previous PR closed, I had to create a new one.

fixes #54

@sungam3r sungam3r changed the title [WIP] C# 8 nullable reference type support C# 8 nullable reference type support Oct 28, 2019
Comment on lines 308 to 315
[Fact]
public void Should_Annotate_OpenGeneric()
{
AssertPublicApi(typeof(StringNullableList<>),
@"namespace PublicApiGeneratorTests.Examples
{
public class StringNullableList<T> : System.Collections.Generic.List<T?>
where T : struct
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the where T : class case the interesting one?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@jnm2 jnm2 left a comment

Choose a reason for hiding this comment

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

Not that it means much, but I can't think of anything else to check. You're awesome, and thanks for all this work!

@sungam3r sungam3r changed the title C# 8 nullable reference type support [WIP] C# 8 nullable reference type support Nov 1, 2019
@danielmarbach
Copy link
Member

I don't think it is a good idea to bundle so many things into the same PR. Let's keep this one focused. I'm ok to take this one with 107 and 108 but let's not add more

@danielmarbach
Copy link
Member

Is this still WIP?

@sungam3r sungam3r closed this Nov 2, 2019
@sungam3r sungam3r deleted the Nullable branch November 2, 2019 12:36
@sungam3r
Copy link
Member Author

sungam3r commented Nov 2, 2019

I did not see your messages. Yes, it was at work, but I see that it is time to finish. I rolled back the last commit related to dynamic support.

@sungam3r
Copy link
Member Author

sungam3r commented Nov 2, 2019

@danielmarbach @jnm2 I made a new (final) PR #115 with squashed commits.

@jnm2
Copy link
Contributor

jnm2 commented Nov 2, 2019

You can always force-push instead of closing the PR and starting a new one. GitHub displays an event describing the force-push with a link to compare before and after.

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.

C# 8 nullable reference type support
3 participants