StackExchange.Redis icon indicating copy to clipboard operation
StackExchange.Redis copied to clipboard

Options: support changing lib-ver

Open NickCraver opened this issue 2 years ago • 4 comments

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.

NickCraver avatar Sep 10 '23 12:09 NickCraver

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

shacharPash avatar Sep 26 '23 08:09 shacharPash

@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.

mgravell avatar Sep 27 '23 15:09 mgravell

@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.

NickCraver avatar Oct 04 '23 12:10 NickCraver

@NickCraver Thanks for updating 👍

shacharPash avatar Oct 05 '23 07:10 shacharPash