Skip to content

Support FieldDeclarationSyntax in CS8618 fixer#45227

Merged
jasonmalinowski merged 9 commits intodotnet:masterfrom
Youssef1313:nullable-cs8618-bug
Jun 23, 2020
Merged

Support FieldDeclarationSyntax in CS8618 fixer#45227
jasonmalinowski merged 9 commits intodotnet:masterfrom
Youssef1313:nullable-cs8618-bug

Conversation

@Youssef1313
Copy link
Member

Fixes #44983

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 16, 2020 11:48
@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jun 16, 2020
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

LGTM with attribute added.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Jun 16, 2020

@CyrusNajmabadi The test is failing. I'll try to debug soon. I'll add the WorkItem attribute when fixing the current failure.

@jasonmalinowski
Copy link
Member

@Youssef1313 Not sure why the test is failing either; the code looks right. Ping me when you fix it up and I'll take a look again.

@Youssef1313
Copy link
Member Author

@jasonmalinowski The test should be passing now.

@Youssef1313
Copy link
Member Author

There is a build failure, but in some other test I don't know about.

@jasonmalinowski
Copy link
Member

@Youssef1313 Looks like that's a random failure with the integration test; I think we need one more fix here, and that failing job will rerun when we push a change.

@CyrusNajmabadi
Copy link
Contributor

LMK when this passes, and i'll merge in.

@Youssef1313
Copy link
Member Author

Not sure what is the reason of the test failure. @CyrusNajmabadi, Can you take a look please?

@jasonmalinowski
Copy link
Member

This test is failing:

Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics.DeclareAsNullable.CSharpDeclareAsNullableCodeFixTests.MultipleDeclarator_NoDiagnostic

I'll make the suggestion in the PR code, one sec.

…reAsNullableCodeFixTests.cs

Co-authored-by: Jason Malinowski <jason@jason-m.com>
Copy link
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Code looks good, just needs the test to be fixed up and this is ready to go in.

@Youssef1313
Copy link
Member Author

@jasonmalinowski Is the following message is the reason for the failure or there is something wrong with the PR itself?

image

@jasonmalinowski
Copy link
Member

@Youssef1313 I've reset the PR; at this point I think that's some flakiness in our infrastructure and this is good to go otherwise.

@jasonmalinowski jasonmalinowski merged commit 16bd662 into dotnet:master Jun 23, 2020
@ghost ghost added this to the Next milestone Jun 23, 2020
@jasonmalinowski
Copy link
Member

Thanks @Youssef1313 for the fix!

@Youssef1313 Youssef1313 deleted the nullable-cs8618-bug branch June 23, 2020 05:32
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CS8618 should provide fixer for making fields nullable

5 participants