Skip to content
This repository has been archived by the owner on Jul 7, 2022. It is now read-only.

Enable compilation across all platforms #156

Merged
merged 1 commit into from
May 9, 2019

Conversation

josephwoodward
Copy link
Contributor

@josephwoodward josephwoodward commented May 8, 2019

Also add Rider to git ignore.

A few tests currently fail when running on a Mac so I'll take a look at them at some point.

@codecov-io
Copy link

codecov-io commented May 8, 2019

Codecov Report

Merging #156 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #156   +/-   ##
=======================================
  Coverage   87.19%   87.19%           
=======================================
  Files          15       15           
  Lines         406      406           
  Branches       82       82           
=======================================
  Hits          354      354           
  Misses         27       27           
  Partials       25       25

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 c96f028...c036ea1. Read the comment docs.

@AnthonySteele
Copy link
Contributor

Could you provide details on why it's better to include Microsoft.NETFramework.ReferenceAssemblies in the NLog.StructuredLogging.Json.csproj file, when it builds without it present; and if it's safe to ship a release using Version="1.0.0-preview.1 of this package

@slang25
Copy link
Member

slang25 commented May 9, 2019

Just to comment on if a preview of ReferenceAssemblies is safe, I would say for most cases yes because the assemblies aren't in preview, the package is, so that means that you won't get weird runtime errors due to compiling against the wrong API surface, it will be either the build works or it fails due to things like filename casing, or some MSBuild problem that mean they don't resolve altogether.

It currently builds in CI as it is finding the mono reference assemblies the build script only builds for netcoreapp2.1, this change means that you don't need anything other than the .NET Core SDK to build all tfms.

@AnthonySteele
Copy link
Contributor

So this is purely a build-time thing? No impact on the shipped package - I don't want to ship a release with a dependency on a -preview.1 .

@slang25
Copy link
Member

slang25 commented May 9, 2019

The PrivateAssets="All" part ensures that this package does not end up becoming an external dependency for the project/package.

@AnthonySteele AnthonySteele merged commit efea0e6 into justeat:master May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants