Skip to content

enable nullable on new project templates#3173

Merged
vlada-shubina merged 2 commits intodotnet:mainfrom
jmarolf:enable-nullable
Jun 3, 2021
Merged

enable nullable on new project templates#3173
vlada-shubina merged 2 commits intodotnet:mainfrom
jmarolf:enable-nullable

Conversation

@jmarolf
Copy link
Copy Markdown
Contributor

@jmarolf jmarolf commented May 16, 2021 •
edited by vlada-shubina
Loading

resolves dotnet/templates#485

fixes #2979

@jmarolf jmarolf requested a review from a team as a code owner May 16, 2021 22:47
@vlada-shubina vlada-shubina added need-pm-discussion Need agreement from PM that the issue aligns to targeted stories for any of the next 2 releases triaged The issue was evaluated by the triage team, placed on correct area, next action defined. labels May 17, 2021
@vlada-shubina
Copy link
Copy Markdown
Member

vlada-shubina commented May 17, 2021

hi @jmarolf, thank you for opening it - we'll need to discuss it with @KathleenDollard prior to merge.

@DavidKarlas
Copy link
Copy Markdown
Contributor

DavidKarlas commented May 17, 2021

I also believe you will need to change template.json as well, just dotnetcli.host.json won't be enough.

@jmarolf jmarolf force-pushed the enable-nullable branch from c51dea3 to 620401a Compare May 18, 2021 15:55
@vlada-shubina
Copy link
Copy Markdown
Member

vlada-shubina commented May 27, 2021

@jmarolf we had a chat on this with @KathleenDollard on Tue, and agreed not to merge it as nullable should not be enabled by default.

@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented May 27, 2021

@vlada-shubina and @KathleenDollard that is not my understanding of where we landed.

vlada-shubina
vlada-shubina previously requested changes Jun 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@vlada-shubina vlada-shubina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@jmarolf
I added some comments inline.

Additional question: is this change intended only for net6.0 TFM? If we need to enable it for other tfms (3.1, 5.0) we need to include same to these templates too.

vlada-shubina
vlada-shubina reviewed Jun 2, 2021
View reviewed changes
@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Jun 2, 2021

is this change intended only for net6.0 TFM? If we need to enable it for other tfms (3.1, 5.0) we need to include same to these templates too.

The intent is that this change is only for .NET 6

@jmarolf jmarolf force-pushed the enable-nullable branch from 2af64ba to 7f9adf7 Compare June 2, 2021 17:31
@jmarolf jmarolf dismissed vlada-shubina’s stale review June 2, 2021 17:32

made requested changes

@vlada-shubina vlada-shubina self-assigned this Jun 2, 2021
This was referenced Jun 3, 2021
Closed
Closed
@vlada-shubina vlada-shubina merged commit c3fc0b9 into dotnet:main Jun 3, 2021
@jmarolf
Copy link
Copy Markdown
Contributor Author

jmarolf commented Jun 3, 2021

Thanks @vlada-shubina!

@dsplaisted dsplaisted mentioned this pull request Jun 17, 2021
Closed
@vlada-shubina vlada-shubina mentioned this pull request Jul 13, 2021
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vlada-shubina vlada-shubina vlada-shubina approved these changes

+1 more reviewer

@KathleenDollard KathleenDollard KathleenDollard left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@vlada-shubina vlada-shubina

Labels

need-pm-discussion Need agreement from PM that the issue aligns to targeted stories for any of the next 2 releases triaged The issue was evaluated by the triage team, placed on correct area, next action defined.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable nullable by default for new projects Change C# templates to have #nullable enabled by default

4 participants

@jmarolf @vlada-shubina @DavidKarlas @KathleenDollard