Skip to content
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

Reduce netstandard version to 1.1 #82

Merged

Conversation

sblom
Copy link
Member

@sblom sblom commented Oct 4, 2020

NOTE: I don't know if this is a good idea or not--I just wanted to kick off the conversation.

I was just experimenting with how low I could get our .NET Standard version.

There are two commits:

  1. The first one lowers our minimum target platform to netstandard1.1, and
  2. The second one enables a bunch of previously conditionally compiled code everywhere, since it now seems to be universally supported by all of our platform targets.

The first one doesn't appear to have any downsides. The second is probably safe as well. It loses ICloneable on Context, but it looks like it was disabled in all of our builds anyway.

Analysis
It turns out that all tests pass with zero changes all the way down to netstandard1.1. Unfortunately, netstandard1.0, is missing HttpClient. Given that netstandard1.0 picks up ZERO additional compatible framework versions, this doesn't seem like a worthwhile tradeoff.

netstandard1.1 covers us all the way down to .NET Core 1.0 & .NET Framework 4.5. Best practices recommend targeting netstandard2.0 as well so that for modern frameworks one doesn't have to restore hundreds of no-op netstandard NuGet packages.

@asbjornu, you added netcoreapp2.1, a couple of years ago. What does that buy us on top of netstandard1.1;netstandard2.0?

@asbjornu
Copy link
Member

asbjornu commented Oct 4, 2020

@asbjornu, you added netcoreapp2.1, a couple of years ago. What does that buy us on top of netstandard1.1;netstandard2.0?

I did that as a part of #38, which was about getting the build to pass on Travis. I can't remember exactly why netcoreapp2.1 was added, but if everything works without it and with the target lowered from netstandard1.3 to netstanndard1.1, I say go for it!

@goofballLogic
Copy link
Member

yes agreed. I'd like to see legacy frameworks supported when we can too. I think it's very achievable for this library.

asbjornu
asbjornu previously approved these changes Oct 5, 2020
@asbjornu
Copy link
Member

asbjornu commented Oct 5, 2020

I suppose this is a backwards compatible change so we can release it as a part of the next 1.1 release?

@asbjornu asbjornu added this to the 1.1 milestone Oct 5, 2020
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #82 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
+ Coverage   68.72%   68.74%   +0.02%     
==========================================
  Files          23       23              
  Lines        4067     4067              
  Branches     1033     1033              
==========================================
+ Hits         2795     2796       +1     
+ Misses       1128     1127       -1     
  Partials      144      144              
Impacted Files Coverage Δ
src/json-ld.net/Core/Context.cs 89.56% <ø> (ø)
src/json-ld.net/Core/JsonLdApi.cs 84.85% <ø> (ø)
src/json-ld.net/Core/JsonLdProcessor.cs 68.68% <ø> (ø)
src/json-ld.net/Core/NormalizeUtils.cs 95.47% <ø> (ø)
src/json-ld.net/Util/JavaCompat.cs 70.00% <ø> (+0.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db11318...6add465. Read the comment docs.

@sblom sblom marked this pull request as ready for review October 5, 2020 20:25
@sblom sblom changed the title EXPLORATION: Reduce netstandard version to 1.1 Reduce netstandard version to 1.1 Oct 5, 2020
@sblom sblom force-pushed the experiment/minimum-netstandard branch from edd25fb to 6add465 Compare October 5, 2020 21:06
@sblom
Copy link
Member Author

sblom commented Oct 5, 2020

Alright. With that last commit bringing in net40 support, we now have support for every framework and PCL profile that this project has ever supported since birth.

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

Awesome!

@asbjornu asbjornu merged commit ff57ebe into linked-data-dotnet:master Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants