Make AddinFileSystemExtension a bit more extensible. #126
Conversation
Allow RequiresIsolation to be set without the need to inherit from the class. Allow overriding CleanupReflector() and make the reflector field protected.
| public virtual bool RequiresIsolation { | ||
| get { return true; } | ||
| } | ||
| public virtual bool RequiresIsolation { get; set; } = true; |
There was a problem hiding this comment.
Isn't this an API break? People overriding need to override the set now too.
There was a problem hiding this comment.
Yup! Curious how many consumers this will break. Not sure how to assess the risk vs. reward here.
There was a problem hiding this comment.
I'm not sure why we need to add a setter. Since this implies that someone will extend AddinFileSystemExtension anyway, why can't the derived class just override the getter behaviour?
There was a problem hiding this comment.
It’s the opposite - Mikayla inherits from this class just to override the setter, and with this change she won’t have to.
There was a problem hiding this comment.
I also subclass to force it to use the Cecil reflector: https://github.com/mhutch/MonoDevelop.AddinMaker/blob/master/MonoDevelop.Addins.Tasks/AddinTask.cs#L80
Allow RequiresIsolation to be set without the need to inherit from the class.
Allow overriding CleanupReflector() and make the reflector field protected.