Skip to content

Conversation

@AArnott
Copy link
Contributor

@AArnottAArnott commented May 8, 2016

Adds .NET Core support (by way of targeting .NET Standard 1.3). .NET Framework 4.0 is still a target, but we
leverage VS2017 .NET SDK projects to multi-target both net40 and netstandard1.3 to maximize reach and enable libgit2sharp to work across platforms by way of .NET Core.

Closes#1293

AArnott added 30 commits March 29, 2016 08:46
This is in preparation for a portable project alongside the net40 one.
Plenty of compiler errors at this point.
Before this, it only existed in debug configuration.
RecurseSubmodulesException claimed to be serializable but was missing a deserializing constructor so I added that.
This will keep the maintenance cost of updating the version to just one file (one line, in fact) instead of the 3 lines it was before, and the dozens of lines it would become with the increase in number of projects that may occur in the near future.
And fix up build breaks it introduces
@AArnott
Copy link
ContributorAuthor

Yes, that makes sense.
nit: I would leave analyzers in PrivateAssets as well, since if analyzers were ever added to the nativebinaries package, it wouldn't be meant to be consumed by libgit2sharp consumers.

@bording
Copy link
Member

nit: I would leave analyzers in PrivateAssets as well, since if analyzers were ever added to the nativebinaries package, it wouldn't be meant to be consumed by libgit2sharp consumers.

So my thought process on this was, since we don't have any analyzers in the native binaries package, there wouldn't be any harm dropping that from the list. I can't currently think of a scenario where we'd add an analyzer, but if we did, who's to say if it would be just for LibGit2Sharp, or all consumers?

Ultimately, regardless of which option we choose, there's a chance it would have to be revisited should the native binaries package actually add an analyzer, so it made sense to me to only include things in the list that we know for sure we don't want downstream consumers to see.

@AArnott
Copy link
ContributorAuthor

That's fine. It was just a nit anyway, and my motivation for it was to maintain the default nuget include/exclude behaviors unless we had an explicit reason to do otherwise.

@AArnott
Copy link
ContributorAuthor

@bording and @ethomson you're not waiting on me, right? I think we're waiting on your review of the PR?
It would be great to get this in, of course. :)

@bording
Copy link
Member

@AArnott No, not waiting on you. I do want to take one more look over this now that all the feedback has been incorporated, but I believe the main thing we're waiting on is that @ethomson wants to get out one more release before we merge this in.

@ethomson
Copy link
Member

I'd like to do a release but we're waiting on finalizing some things in libgit2. I'm going to branch for the upcoming release so that we can merge this into master.

@Alxandr
Copy link

@ethomson Do you have a link to any issue on things being finalized in libgit2?

@stephenwilliams
Copy link

Is there any indication when this will be available?

@ethomson
Copy link
Member

Okay - v0.24 is released based on libgit2 v0.26. Thanks for your patience.

@AArnott would you mind a final update just to fix the merge conflicts (which I think should be minor, but if not, then please let me know and I'm happy to resolve them), and rev the version number, and then we'll merge this?

@AArnott
Copy link
ContributorAuthor

with pleasure...

@AArnottAArnott changed the title Add a portable library version (for CoreCLR support)Target .NET Standard 1.3Jun 21, 2017
@AArnott
Copy link
ContributorAuthor

Validation complete.

@ethomsonethomson merged commit c72b740 into libgit2:masterJun 24, 2017
@ethomson
Copy link
Member

Thanks @AArnott, @bording and everybody for your hard work and infinite patience! This is amazing! 🎉

@ethomson
Copy link
Member

image

@ethomson
Copy link
Member

giphy

@ethomson
Copy link
Member

giphy 1

@jaredcnance
Copy link

This is great and thanks for your work. Any idea when 0.25 will be available on NuGet?

@bording
Copy link
Member

@jaredcnance No specific ETA yet, but hoping to have available "Soon".

@kzukzu mentioned this pull request Jul 27, 2017
@firelizzard18firelizzard18 mentioned this pull request Jan 25, 2018
@Cyberboss
Copy link

(tm)

Sign up for freeto 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.

12 participants

@AArnott@ethomson@shiftkey@ah-@devlead@bording@ctaggart@danwalmsley@Alxandr@stephenwilliams@jaredcnance@Cyberboss