Conversation
…n-PackageItemViewModelTests
|
bump |
|
This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 30 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch. |
zivkan
left a comment
There was a problem hiding this comment.
@SimonCropp This PR doesn't have anything that would cause me to ask for changes or block it as conceptually unwanted. Indeed, having all our code use nullable checks would help reduce risk of NullReferenceException bugs in the product.
But this PR enables nullable for a test file, while the product code continues to not have null checks. At best this PR does not help improve null checking in the product, and at worst it might make some people assume the product code is being null checked, when it's not.
In the past, when I felt less pressure to get other work done, I was spending tens of hours enabling nullable checks for whole projects at a time (1, 2, 3, 4). It's a lot of painful work, so I understand if an external contributor doesn't want to make the effort. Otherwise, I'd really prefer to see individual files enabling nullable when other work is already being done on them.
This repo has ~4200 *.cs files. Some of them already have nullable enabled, but most don't yet. I really don't want to see hundreds, let alone thousands, of PRs each null annotating one file.
Bug
Fixes: NuGet/Home#14434
Description
PR Checklist