Enable Nullable ReferenceTypes #2236
Conversation
stefannikolei
changed the title
| @@ -148,7 +148,7 @@ public static Memory<TPixel> DangerousGetPixelRowMemory<TPixel>(this ImageFrame< | |||
| Guard.MustBeGreaterThanOrEqualTo(rowIndex, 0, nameof(rowIndex)); | |||
| Guard.MustBeLessThan(rowIndex, source.Height, nameof(rowIndex)); | |||
|
|
|||
| return source.PixelBuffer.GetSafeRowMemory(rowIndex); | |||
| return source.PixelBuffer!.GetSafeRowMemory(rowIndex); | |||
There was a problem hiding this comment.
@JimBobSquarePants Should I look for all places where I used ! and add an ArgumentNullException.ThrowIfNull?
|
@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 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 - 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 |
|
Thanks for understanding @stefannikolei |
|
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 |
#2231