files-community / Files

Building the best file manager for Windows
https://files.community
MIT License
32.37k stars 2.07k forks source link

Feature: Nullable reference types annotations #9895

Closed hez2010 closed 1 week ago

hez2010 commented 1 year ago

We are migrating to WASDK and adopt .NET 6, so we get build-in nullable reference types support.

Requirements

This is a large one, but we can do it progressively.

yaira2 commented 1 year ago

Good idea, I believe we can do this one by one (e.g. update things as you notice them).

hecksmosis commented 1 year ago

According to stackoverflow and the Microsoft docs, the way to eliminate the warnings with "possible" is to mark the properties as nullable or to check for null values

marcelwgn commented 1 year ago

My 2 cents: If something can be null, we should make it nullable and add null checks where they are used. If the property can never be null (for reasons that the compiler cannot know about), then disabling the warning for that property with an explanation why is the best way.

yaira2 commented 1 year ago

I think that's a fair solution. That being said, these warnings should be fixed as needed because otherwise, opening a pull request just to fix the warnings takes away from other review efforts.

marcelwgn commented 1 year ago

So opening a PR just to fix these warnings is not encouraged?

hecksmosis commented 1 year ago

I think not

yaira2 commented 1 year ago

So opening a PR just to fix these warnings is not encouraged?

It can also impact stability to make large changes like this without understanding the context.

yaira2 commented 6 months ago

Just to clarify my previous comments, opening PRs for this that are limited in scope (eg one or two classes) are fine.

0x5bfa commented 1 week ago

As this might have to be done gradually and not in Separated PRs, can’t we merge this to #4180?