You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
Jimmy Campbell (@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.
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
Activity
jimmyca15 commentedon Aug 2, 2024
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 commentedon Aug 5, 2024
Jimmy Campbell (@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 commentedon Aug 5, 2024
But we never know.
zhiyuanliang-ms commentedon Aug 15, 2024
Discussed offline. Will not take the breaking change in v4
WeihanLi commentedon Aug 19, 2024
For public class types, think we could always start with
sealedtypes if applicable since change a type fromsealedto non-sealed would not be a breaking change, for internal types, maybe we could enable theCA1852analyzer rule in the editorconfig file to help enforce thesealedinternal types rulehttps://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1852
https://github.com/microsoft/FeatureManagement-Dotnet/blob/main/.editorconfig