Skip to content

Nullable reference type for internal code #19513

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
smitpatel opened this issue Jan 7, 2020 · 3 comments
Closed

Nullable reference type for internal code #19513

smitpatel opened this issue Jan 7, 2020 · 3 comments

Comments

@smitpatel
Copy link
Contributor

smitpatel commented Jan 7, 2020

Seems like annotating out internal APIs for NRT is doing more than just converting NotNull to non-nullable reference type. We should better define set of guidelines about it.

Specifically, if we believe that current NotNull annotations are not correct then we should correct them first.

I prefer to err on the side of keeping things nullable rather than making everything is non-nullable just because we can.

@roji
Copy link
Member

roji commented Jan 7, 2020

Let's discuss, but I think that as we see opportunities for improvement/cleanup there's no reason to artificially abstain. Each change can be discussed with the expert on a case-by-case basis just like always.

@smitpatel
Copy link
Contributor Author

No one is artificially abstaining any change. Just not a fan of random changes based on personal preferences.

@roji
Copy link
Member

roji commented Jan 8, 2020

Decisions from design meeting (feel free to complete if I've left anything out):

  • As a general rule, try to avoid introducing breaking changes for API nullability cleanliness (unless there's special value, the API is rarely-used, etc.). If done, explicitly call out any such cases.
  • Continue using null to mean "default", where users don't want to specify a value (e.g. coalesce nulls to empty lists where relevant).
  • Continue using null to mean that a value is absent or unknown, rather than looking for other representations (e.g. index name being an empty string)
  • Keep the typical consumer API usage in mind when deciding if something is nullable or not. This is important for cases where some value can be valuable during some initialization/construction phase, but is guaranteed to be non-nullable after that phase has completed (e.g. model building). In those cases it seems that non-nullable is the way to go, even if during model-building only null is observable.

@roji roji closed this as completed Jan 8, 2020
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants