-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add explicit netstandard2.0 support #39
Conversation
TraivsCI seems to be disabled atm for this repo: https://travis-ci.org/maxmind/MaxMind-DB-Reader-dotnet. |
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.
Thanks! This looks great as always. I had one question about the AppVeyor config. Travis should be re-enabled now; I am not sure what happened there.
Sorry for the delay in review; I was on holiday.
@@ -17,13 +17,17 @@ environment: | |||
TEST_FRAMEWORK: net452 | |||
LIB_FRAMEWORK: net45 | |||
- CONFIGURATION: Debug | |||
CONSOLE_FRAMEWORK: netcoreapp1.0 | |||
LIB_FRAMEWORK: netstandard1.4 | |||
TEST_FRAMEWORK: netcoreapp1.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.
Is there a reason the debug test build for 1.4 was removed?
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 was thinking if it makes sense to only configure the build+release jobs for the latest framework and release for older ones.
TravisCI seems to still have some issue here https://travis-ci.org/maxmind/MaxMind-DB-Reader-dotnet/branches, but working against my fork https://travis-ci.org/am11/MaxMind-DB-Reader-dotnet. Maybe in repo settings there, the PR trigger is off: |
Weird. "Build pull request updates" was enabled. I deleted the integration from GitHub and then re-enabled it again in Travis. Hopefully it should work now. Could you try pushing something? |
Although netstandard1.x libs are compatible with higher version of standards, it is a good idea to target each standard specifically if the dependencies are different. In case of NETSTANDARD 2.0, we don't need additional dependencies other than the default `NETStandard.Library` which is a [flat package](https://stackoverflow.com/a/43999545/863980). Also removed the mono-specific solution and project files, as the latest Mono uses CSC (C# Roslyn compiler from dotnet/roslyn repo), instead of MCS and MSBuild (from microsoft/MSBuild repo) instead of xbuild. See http://www.mono-project.com/docs/about-mono/releases/5.0.0 for details.
Yay, it worked! 🌮 |
@oschwald, with dotnet cli 2.0, if we have full mono or only reference assemblies of some desktop TFM (say net45, net32, net20) installed on Unix, then we can use dotnet |
Thanks! |
Also removed Mono specific SLN/csprojs files for unification. Related to maxmind/MaxMind-DB-Reader-dotnet#39.
Add explicit netstandard2.0 support
Also removed Mono specific SLN/csprojs files for unification. Related to maxmind/MaxMind-DB-Reader-dotnet#39.
Although netstandard1.x libs are compatible with higher version of
standards, it is a good idea to target each standard specifically
if the dependencies are different. In case of NETSTANDARD 2.0,
we don't need additional dependencies other than the default
NETStandard.Library
which is aflat package.
Also removed the mono-specific solution and project files, as the
latest Mono uses CSC (C# Roslyn compiler from dotnet/roslyn repo),
instead of MCS and MSBuild (from microsoft/MSBuild repo) instead of
xbuild. See http://www.mono-project.com/docs/about-mono/releases/5.0.0
for details.