StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.87k stars 1.51k forks source link

Options: support changing lib-ver #2547

Open NickCraver opened 12 months ago

NickCraver commented 12 months ago

This adds similar support from LibraryName to LibraryVersion so that consumers can override both if they so choose - this works in all the same places, DefaultOptionsProvider, etc.

Resolves #2533.

shacharPash commented 11 months ago

@NickCraver Is there an estimate of when it is going to be merged?

mgravell commented 11 months ago

@shacharPash I believe the snag on this one is the static -> virtual, which is a big change to make in a minor; in voice call, we discussed options here, including breaking the change into two parts, so that there's no single hard break - for example, in version X we could do:

- protected static string LibraryVersion => Utils.GetLibVersion();
+ [Obsolete("words, basically: stop using this")]
+ protected static string LibraryVersion => DefaultLibraryVersion;
+ protected static string DefaultLibraryVersion => Utils.GetLibVersion();

then in change Y

- [Obsolete("words, basically: stop using this")]
- protected static string LibraryVersion => DefaultLibraryVersion; 
+ public virtual string LibraryVersion => DefaultLibraryVersion;

If we do go that route, it'll take at least 2 deploys to get it released. However, I'm open to other options. Just; the hard break here is not ideal, even if it is a very niche API that most folks won't be using.

The other option, of course, is to just use a different name, i.e.

  protected static string LibraryVersion => Utils.GetLibVersion();
+ public virtual string EffectiveLibraryVersion => LibraryVersion;

Which is a little ugly, but much quicker to ship.

NickCraver commented 11 months ago

@shacharPash After talking about this and looking through it - I believe it will be a 3.x addition due to the need for a break here. We have many calls to Utils directly to get this (that can't necessarily use options), so having an intermediate non-breaking solution which we'd still not want to break for 3.x either is pretty messy.

shacharPash commented 11 months ago

@NickCraver Thanks for updating 👍