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

Ported to .Net 3.5 #31

Merged
merged 7 commits into from Apr 7, 2016

Conversation

Projects
None yet
6 participants
@mcatanzariti
Copy link

commented Oct 30, 2015

I ported Open.Nat back to .Net 3.5 based on Microsoft official backport of the Task Parallel Library.

I introduced two precompilation symbols (NET35 and NET45)
I added projets and solutions for Net 3.5 and suffix them with Net35
I renamed existing projects and solutions to suffix them with Net45

Most of the work consisted in adding a .Net 3.5 version of async methods by replacing the await keywork by continuations tasks (Task.ContinueWith).

I also added a few extensions in Extensions.cs to emulate missing methods in .Net 3.5 or in the backported TPL:

  • EnumExtension.HasFlag
  • CancellationTokenSourceExtension.CancelAfter
  • TaskExtension.Delay
  • TaskExtension.WhenAny
  • TaskExtension.WhenAll

The existing .Net 4.5 code should be unchanged.

Oh and I also tested Open.Nat 3.5 with Unity3d 5.2 and it seems to work perfectly!

mcatanzariti added some commits Oct 24, 2015

Renamed projects and solutions to suffix them with Net45
Added projets and solutions for Net 3.5 and suffix them with Net35
fixed references after project renaming
added nugget package PortableTPL.dll
@rapgro

This comment has been minimized.

Copy link

commented Oct 30, 2015

👍

@lontivero

This comment has been minimized.

Copy link
Owner

commented Oct 31, 2015

@mcatanzariti this looks really awesome and finishes with the biggest problem Open.NAT has: too new .NET version. This comment is just to let you know I've seen the PR and i am going to review it and test it (what will take me several days).

👍 👍 👍 👍

mcatanzariti added some commits Oct 31, 2015

Revert useless change in ISearcher
Put back .Net 4.5 implementations that have been replace by .Net 3.5 implementations
Fixed indentation
@mcatanzariti

This comment has been minimized.

Copy link
Author

commented Oct 31, 2015

Reverted useless change in ISearcher …
Put back .Net 4.5 implementations that have been replaced by .Net 3.5 implementations
Fixed indentation

and I tried to rebase all my changes in one commit.

@Mailaender

This comment has been minimized.

Copy link

commented Nov 22, 2015

You can use https://help.github.com/articles/about-git-rebase to squash the commits.

@@ -101,6 +101,24 @@ internal static string ToPrintableXml(this XmlDocument document)
}
}

#if NET35
public static Task<TResult> TimeoutAfter<TResult>(this Task<TResult> task, TimeSpan timeout)

This comment has been minimized.

Copy link
@codand

codand Feb 18, 2016

Currently this method always throws a TimeoutException when compiled in release mode (trying to use this with Unity). Is this pull request still alive?

This comment has been minimized.

Copy link
@mcatanzariti

mcatanzariti Feb 18, 2016

Author

I will take a look. Thank you for the report!

@mcatanzariti

This comment has been minimized.

Copy link
Author

commented Feb 26, 2016

@codan Has my fix resolved your problem in Unity?

@mcatanzariti

This comment has been minimized.

Copy link
Author

commented Feb 26, 2016

@lontivero Do you still plan to review this pull request?

@codand

This comment has been minimized.

Copy link

commented Feb 27, 2016

@mcatanzariti It appears to be working now. Thanks for the fix!

@lontivero

This comment has been minimized.

Copy link
Owner

commented Feb 27, 2016

@mcatanzariti yes, I still plan to review this pull request. I have been busy but I have this in my todo list.

@AnsonKindred

This comment has been minimized.

Copy link

commented Mar 31, 2016

Just wanted to say that I'm using Open.NAT with this change included in a Unity plugin that I'm awaiting approval on now and so far everything has been working perfectly. Thanks @mcatanzariti and @lontivero

@lontivero lontivero merged commit b12bd8d into lontivero:master Apr 7, 2016

@mcatanzariti

This comment has been minimized.

Copy link
Author

commented Apr 7, 2016

@lontivero Good news :-D

@lontivero

This comment has been minimized.

Copy link
Owner

commented Apr 7, 2016

I didn't want to do it in this way because there are things that i wanted to do first:

  • To create a nuget package with two targets,
  • To fix the CI process that doesn't know how to build the new solutions and,
  • To consolidate all in one solution (if possible) doing something like this.

Sadly I had no time to do that and many people need this PR so, i didn't want to keep it in a PR forever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.