Skip to content

Switch to dot net standard#150

Closed
darrencauthon wants to merge 21 commits into
masterfrom
switch_to_dot_net_standard
Closed

Switch to dot net standard#150
darrencauthon wants to merge 21 commits into
masterfrom
switch_to_dot_net_standard

Conversation

@darrencauthon

Copy link
Copy Markdown
Owner

I have to see this work for myself, as I have a few other libraries that are going to have to be moved to standard.

I'm following @asherber 's work on #138.

@asherber

Copy link
Copy Markdown
Contributor

I'm out of the country for the next 10 days with limited access, so I can't follow along with what you're doing. But FWIW, in VS2017 I have found it much easier to have a single multitargeted project rather than separate projects.

@asherber

asherber commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

I'm back -- can I help with this?

@Jetski5822

Copy link
Copy Markdown
Contributor

Can you upgrade the newtonsoft json version?
then target netstandard1.0
then ditch all the GetTypeInfo stuff

@asherber

asherber commented Jun 1, 2018

Copy link
Copy Markdown
Contributor

@darrencauthon This is heating up for me again. Can I help move things along?

@darrencauthon

Copy link
Copy Markdown
Owner Author

@asherber

Let me take a look tonight.

The library itself is safe to go, and I've learned enough about testing with dotnetstandard to get the tests ported over. The deployment story is one I'm going to have to learn -- I've been doing dot net nuget deployments for years, but never for dot net & dot net core. Do you know much about nuget and core?

@asherber

asherber commented Jun 1, 2018

Copy link
Copy Markdown
Contributor

I was just looking at the branch. I would strongly suggest making the project multi-targeted, targeting both net45 and netstandard1.6. Even though someone writing a Framework project can consume the netstandard lib, it seems to be common to maintain a framework-only assembly when you had one previously. I can take a look at this tonight; it will probably require just a few conditional compiles. Then the testing assembly becomes multitargeted as well (netcoreapp2.0 and net45) and can test both sides of the code. I've done this with xUnit but not yet with NUnit.

In terms of deployment, I haven't worked with travis, but once you've got your csproj set up, all you do is dotnet pack -c Release to create the single NuGet package with net45 and netstandard inside. All of the attributes that are currently in the nuspec file should be moved to the csproj; I can help with that as well.

I'm a little sorry to see the MailMessage stuff go. Would you consider leaving it in for the net45 build and also adding a netstandard2.0 build that includes it? (2.0 includes System.Net.Mail) You could just use conditional compiles for netstandard1.6 to remove it from that build.

@asherber

asherber commented Jun 1, 2018

Copy link
Copy Markdown
Contributor

I just sent a PR to this branch that multi targets (net45 and standard1.6) the main assembly and the tests. All tests are passing locally.

I would suggest adding netstandard2.0 to this as well, even if you don't want to bring back the MailMessage stuff. I suspect that most new development is going to be on 2.0 rather than on an older version, and having a package that targets 2.0 means not having the dependency on NETStandard.Library.

@darrencauthon

Copy link
Copy Markdown
Owner Author

@asherber I fell asleep after TKD practice on Friday night. Super-early on Saturday morning, we here in KC were rocked by a very windy storm that shook my house and took out power for a while (http://www.kctv5.com/story/38331875/thousands-without-power-in-kansas-city-following-severe-weather). Sunday was recovery and when I have my kids....

I think I'll be free tonight!

@asherber

asherber commented Jun 4, 2018

Copy link
Copy Markdown
Contributor

I didn't want to cause a bottleneck, but after you've had a chance to look at the PR #161 I have a couple more commits queued up that add netstandard2.0 and reinstate the MailMessage stuff.

@chrisstewart

Copy link
Copy Markdown

Any chance this will get picked back up?

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.

4 participants