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.

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

enable nullable on new project templates #3173

Merged
merged 2 commits into from
Jun 3, 2021
Merged

enable nullable on new project templates #3173

merged 2 commits into from
Jun 3, 2021

Conversation

jmarolf
Copy link
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
Member

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

@DavidKarlas
Copy link
Contributor

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

@vlada-shubina
Copy link
Member

@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
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
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.

...Common.ProjectTemplates.6.0/content/ClassLibrary-CSharp/.template.config/dotnetcli.host.json Show resolved Hide resolved
...Common.ProjectTemplates.6.0/content/ConsoleApplication-CSharp/.template.config/template.json Outdated Show resolved Hide resolved
@jmarolf
Copy link
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 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
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

@KathleenDollard KathleenDollard KathleenDollard left review comments

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

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