Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Make AddinFileSystemExtension a bit more extensible. #126

Closed
wants to merge 1 commit into from
Closed

Make AddinFileSystemExtension a bit more extensible. #126

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions Mono.Addins/Mono.Addins.Database/AddinFileSystemExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace Mono.Addins.Database
[Serializable]
public class AddinFileSystemExtension
{
IAssemblyReflector reflector;
protected IAssemblyReflector reflector;

/// <summary>
/// Called when the add-in scan is about to start
Expand Down Expand Up @@ -201,22 +201,18 @@ public virtual void DeleteFile (string filePath)
File.Delete (filePath);
}

internal void CleanupReflector()
protected internal virtual void CleanupReflector()
{
var disposable = reflector as IDisposable;
if (disposable != null)
disposable.Dispose ();
using (reflector as IDisposable) { }
}

/// <summary>
/// Gets a value indicating whether this <see cref="Mono.Addins.Database.AddinFileSystemExtension"/> needs to be isolated from the main execution process
/// </summary>
/// <value>
/// <c>true</c> if requires isolation; otherwise, <c>false</c>.
/// </value>
public virtual bool RequiresIsolation {
get { return true; }
}
public virtual bool RequiresIsolation { get; set; } = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this an API break? People overriding need to override the set now too.

Copy link
Member Author

@KirillOsenkov KirillOsenkov Nov 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! Curious how many consumers this will break. Not sure how to assess the risk vs. reward here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s the opposite - Mikayla inherits from this class just to override the setter, and with this change she won’t have to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}