search

microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.05k stars 115 forks source link

Should TargetingContext be sealed? #486

Open zhiyuanliang-ms opened 3 months ago

jimmyca15 commented 3 months ago

Yes, it should've been sealed given there was no intent to use it as a base class. However, sealing it now would be a breaking change that I don't believe we have a reason to take.

zhiyuanliang-ms commented 3 months ago

@jimmyca15 In v4 major version bump, we can have breaking change. I don't think there will be anyone inherit TargetingContext. So, v4 is a good chance for us to make it sealed.

jimmyca15 commented 3 months ago

I don't think there will be anyone

But we never know.

zhiyuanliang-ms commented 3 months ago

Discussed offline. Will not take the breaking change in v4

WeihanLi commented 3 months ago

For public class types, think we could always start with sealed types if applicable since change a type from sealed to non-sealed would not be a breaking change, for internal types, maybe we could enable the CA1852 analyzer rule in the editorconfig file to help enforce the sealed internal types rule

[*.{cs,vb}]
dotnet_diagnostic.CA1852.severity = none

https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1852 https://github.com/microsoft/FeatureManagement-Dotnet/blob/main/.editorconfig