-
Notifications
You must be signed in to change notification settings - Fork 108
Conversation
@@ -4,7 +4,7 @@ | |||
|
|||
<PropertyGroup> | |||
<IncludeBuildOutput>false</IncludeBuildOutput> | |||
<TargetFramework>netstandard1.6</TargetFramework> | |||
<TargetFramework>netcoreapp2.0</TargetFramework> |
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.
I had to bump this to netcoreapp2.0 since M.A.Diagnostics.Identity.Service targets net46/netcoreapp2.0. Any reason why this package is not targeting netstandard1.x? @javiercn
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.
We need Certificate Signing APIs that are only available in .NET Core 2.0
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.
I think this is fine, but I'd like @DamianEdwards to confirm this is ok. This means the metapackage can't be used on .NET Framework, but I think for 2.0 that's fine.
LGTM (The identity service part) |
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.
The change looks good to me, but need others to sign off as well.
37c8ebc
to
c6fcd74
Compare
Clarified issues with @DamianEdwards offline in person. |
Addresses #21 #39
I'm not able to add Microsoft.CodeAnalysis.Razor.Workspaces however since it has a PCL dependency: Microsoft.Composition 1.0.27
cc @Eilon @DamianEdwards