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

Change msbuild property to "Nullable" #35432

Closed
jcouv opened this issue May 2, 2019 · 2 comments · Fixed by #35516
Closed

Change msbuild property to "Nullable" #35432

jcouv opened this issue May 2, 2019 · 2 comments · Fixed by #35516

Comments

@jcouv
Copy link
Member

jcouv commented May 2, 2019

From LDM today (5/1/2019), we landed on Nullable rather than NullableContextOptions or NullableReferenceTypes.
The property should be a string, since it will accept "enable", "disable" and a couple other values.

Tagging @KathleenDollard @rainersigwald to confirm the name is okay in the msbuild ecosystem.

@jcouv
Copy link
Member Author

jcouv commented May 4, 2019

@rainersigwald Do you see any issue with naming this property "Nullable"?
Previously, we picked longer names thinking we should avoid conflicts, but Jared found no github project referencing a property named "Nullable". LDM preferred "Nullable" because it's easier to remember.

@rainersigwald
Copy link
Member

No objection here. It's impossible to guarantee that someone somewhere won't already be using a property named Nullable, but I didn't turn up anything widespread on GitHub either, so the risk seems acceptable.

The only other thing I'd mention is that other project types use MSBuild, including C++. Having names that refer to broad concepts that apply in multiple languages/compilers/tools increases the possibility of user confusion when it doesn't work in $non_csharp_project_type_x. But that's not blocking. I think Nullable is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants