Remove #nullable enable

[hishamco]

Seems the nullable attribute was added by Rider, no need for it while it's configured in .csproj

[MikeAlhayek]

Why removing this?

[hishamco]

Why removing this?

Check the issue description, moreover is makes the code-based inconsistent, I'm able to add the settings in the .csproj but the build will fail because we didn't use nullable before

Removing this easier than applying the nullable over the code-base

[MikeAlhayek]

I don't think you should do this. At some point we may want to use nullable everywhere. Since it's already there, I would rather keep it

[hishamco]

At some point we may want to use nullable everywhere

We can do it now if we do so, but no need to use the attribute anymore, using the csproj property is much better and cleaner

[MikeAlhayek]

I would not proceed with this

[hishamco]

Let's hear @sebastienros feedback, we already started months ago to make things consistent, so either we accept nullable or not but not using attribute

[gvkries]

I'm against removing the ones we already have. If we completely want to enable nullable annotations some time in future, this is work that is already done. Enabling nullable for the whole project is also not an options, if only parts are done yet.

[Piedone]

Agree with not doing this. We'll want to fully enable nullable in the future. Having a bit done now will help with that.

[hishamco]

We'll want to fully enable nullable in the future

If we do so it's much better and cleaner to do it in .csproj as I said earlier

[Piedone]

Yes, there's no disagreement there (or rather, it should be done solution-wide in a props file). But until then, we shouldn't revert nullable in those files that already have it, so then we have to reimplement these later.

[sebastienros]

We could have reverted if all the types were marked with ?, but I don't think it's the case in this file.

it should be done solution-wide

That's the theory. That would be a pain though to do it correctly. Also would break all existing PRs. Could be done by modules or groups, could still impact lots of related projects though.

[gvkries]

I think it makes sense to add nullable incrementally on a per file and/or module basis. It is just too much work to do it all at once. Then we can remove the pragmas per project step-by-step as well.

[hishamco]

It could be done module by module as Seb refers to, but I suggest doing it before 3.0.0. Later such things will not acceptable because it will introduce a breaking changes

[Piedone]

OK, but the point is, this PR isn't needed.

[hishamco]

I will close this PR and file issue for tracking