-
Notifications
You must be signed in to change notification settings - Fork 15
Description
I want to rename EnableClientSideEvaluation to EnableLocalEvaluation for consistency with every other Flagsmith SDK. This is currently impossible without introducing breaking changes:
The FlagsmithClient constructor has explicit ordered parameters. We can only add optional parameters at the end, and not rename any existing parameters to avoid breaking changes. This constructor is the main one used in our docs. Customers might be using named parameters, but nothing prevents them from using ordered parameters like we do in our own constructor which exclusively uses ordered parameters.
We do have another constructor with a dedicated configuration object which also has a second HttpClient parameter, for some reason I don't understand, instead of being incorporated into the FlagsmithConfiguration object. Using this constructor is slightly less ergonomic than the previous one, but gives us more flexibility to manipulate the public API for initialising clients:
new FlagsmithClient("ser.my-api-key", enableClientSideEvaluation: true, ...)
// vs
new FlagsmithClient(new FlagsmithConfiguration
{
EnvironmentKey = "ser.my-api-key"
});This constructor is currently only referenced in the "Singleton Initialisation" section on the same docs.
To avoid duplicating parameter documentation and having redundant constructors, we should introduce a new Flagsmith(IFlagsmithConfiguration) constructor, and deprecate all others. Later we can add a Builder pattern to construct clients, which will be backwards compatible with the new single-argument constructor, and is slightly less verbose than manually instantiating a FlagsmithConfiguration object.