SixLabors
Enable Nullable ReferenceTypes
@JimBobSquarePants I think you could have a first look. Let me know what you think and what parts should be changed.
Hmm... there feels like there is way too much changed in this PR, this doesn't feel like the right way to tackle adding NRT into the codebase, it feeld like it should be a much more targeted, file by file/section by section changed, there is way to much expert knowledge required to apply some of these fixes globally, just deciding what should be nullable Vs having more sensible defaults to just changing {get; set;} to {get; init;} there is no one pattern that's right so each change needs review in detail..
I would propose we close this huge PR (sorry about that @stefannikolei ) and instead replace it with a first PR of enable the csproj setting but inline opt out all files with warnings. Then, as a series of follow up PRs, the various sections can be tackled.
This should mean any new files/code will have it enabled by default and we can be more methodical in applying the right fixes as we go.
PS. We should also aim to tackle the public API surface first before diving into the internals.
It's with a heavy heart that I have to agree with @tocsoft here.
I started a review of the PR and while there is some low hanging fruit - Equals( object) - there are several places where we are going to have to significantly refactor the codebase in order to achieve safe nullability handling.
This is my fault. I should have investigated further before encouraging anyone to attempt a single PR.
I'm so, so sorry that you have made so much effort here with so little result.
It's with a heavy heart that I have to agree with @tocsoft here.
I started a review of the PR and while there is some low hanging fruit -
Equals( object)- there are several places where we are going to have to significantly refactor the codebase in order to achieve safe nullability handling.This is my fault. I should have investigated further before encouraging anyone to attempt a single PR.
I'm so, so sorry that you have made so much effort here with so little result.
Its ok. NRT is not an easy change. And it gets big really fast. And errors are there to be done :)
I would change it back to draft. Perhaps it is useful for anyone to look up when making the changes in smaller steps
Sorry I have broken so much! It was really important to get #2276 merged in as it was blocking development.
@JimBobSquarePants this is the old one. I will close it. To not confuse anyone.
I will merge your changes in my branch and fix the problems