Nullable Reference Types for main library by NickCraver · Pull Request #2041 · StackExchange/StackExchange.Redis (original) (raw)

Hey @NickCraver,

I went through the rest of what was in here, had a couple of questions about things that were null and a few other minor comments.

There is an awful lot of what looks like code cleanup changes in the Wrapper classes.

I scrolled through the entirety of IDatabase and IServer, for potentially missing threads to pull on, I only found a few inconsistencies in there (my questions in IServer are fleshed out in the other comments.

But a few things in IDatabase:

• KeyRandom can return a null key if Redis is empty
• It looks Like the HashEntry[] result for HashGetAll MIGHT be null? - the Parse method in the ResultProcessor - looks like it can pull back a null array.
• Is the result of IdentifyEndpoint potentially null?

Overall looks really good, obviously, there are questions as to whether or not this would be a minor or major version revision for the library. Putting that aside, I don't think I saw anything that I would insist on changing.

Also: think it's missing release notes :)