-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/netcore #195
Feature/netcore #195
Conversation
Lens/Compiler/Context.Compilation.cs
Outdated
@@ -117,6 +119,7 @@ private void FinalizeAssembly() | |||
if (!curr.Value.IsImported) | |||
curr.Value.TypeBuilder.CreateType(); | |||
|
|||
#if !NET_CORE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consistently use #if NET_CLASSIC
here? Or is it intended as-is?
/// <summary> | ||
/// Throws an error if the assembly is marked as requiring save-to-disk. | ||
/// </summary> | ||
private void EnsureNotSaving() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to mark it as [Conditional("NET_CLASSIC")]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't work, since Conditional
requires the body to compile, but in NET_CORE
the AllowSave
does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry, that's true. Okay then!
Lens/Lens.csproj
Outdated
|
||
<PropertyGroup> | ||
<TargetFrameworks>net45;net451;net452;net46;net461;net462;net47;netcoreapp2.0</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need anything but net45;netcoreapp2.0
here? Are we using any additional benefits of newer frameworks?
private IEnumerable<Assembly> GetLoadedAssemblies() | ||
{ | ||
var asms = AppDomain.CurrentDomain.GetAssemblies(); | ||
return asms.Where(x => !x.FullName.StartsWith("System.Private.")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly looks like a bit of hack. Probably we need more explanation here at least. What bad consequences will we have if we return System.Private.Smth
from this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to #196.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good job, I approve that.
No description provided.