Skip to content

Add netcoreapp build #1418

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.

Sign up for GitHub

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

Merged
olevett merged 3 commits into from
Aug 26, 2017
Merged

Add netcoreapp build #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e134808
Add netcoreapp build.
jkoritzinsky Aug 8, 2017
b458382
Cleaned up registrations of main schedulers.
Aug 8, 2017
730ff8b
Add empty property group for netcoreapp1.0
Aug 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Directory.build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
<PropertyGroup Condition="'$(TargetFramework)' == 'MonoAndroid70'">
<DefineConstants>$(DefineConstants);MONO;ANDROID</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition="'$(TargetFramework)' == 'netcoreapp1.0'">
</PropertyGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'uap10.0' ">
<PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform " Version="5.2.2" />
Expand Down
2 changes: 1 addition & 1 deletion src/ReactiveUI/Platforms/net45/PlatformRegistrations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public void Register(Action<Func<object>, Type> registerFunction)
{
registerFunction(() => new ComponentModelTypeConverter(), typeof(IBindingTypeConverter));
RxApp.TaskpoolScheduler = TaskPoolScheduler.Default;
RxApp.MainThreadScheduler = new WaitForDispatcherScheduler(() => DispatcherScheduler.Current);
RxApp.MainThreadScheduler = DefaultScheduler.Instance;
}
}
}
18 changes: 18 additions & 0 deletions src/ReactiveUI/Platforms/netcoreapp1.0/PlatformRegistrations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MS-PL license.
// See the LICENSE file in the project root for more information.

using System;
using System.Reactive.Concurrency;

namespace ReactiveUI
{
public class PlatformRegistrations : IWantsToRegisterStuff
{
public void Register(Action<Func<object>, Type> registerFunction)
{
RxApp.TaskpoolScheduler = TaskPoolScheduler.Default;
Copy link
Member

@olevett olevett Aug 8, 2017

Choose a reason for hiding this comment

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

I'm a little wary of this for a couple of reasons

I'd be interested to know if there are problems if this line is omitted, or if it's fine without. I think I'd keep the PlatformRegistrations class however as it adds some value on it's own.

I also think it might be worth wiring up an IPlatformOperations even if it just returns null, as I think there are cases it would cause a warning to be logged otherwise (@ghuntley may disagree though...)

Copy link
Contributor Author

@jkoritzinsky jkoritzinsky Aug 8, 2017

Choose a reason for hiding this comment

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

  • It looks like it is. Should I remove that setting from all of the platforms?
  • I'll set it to DefaultScheduler.Instance

The only place that logs a warning about a missing IPlatformOperations looks to be in RoutedViewHost, which is only available in builds using the Platforms/windows-common files.

Copy link
Member

@olevett olevett Aug 26, 2017

Choose a reason for hiding this comment

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

I think don't worry about removing it everywhere for now, I think separate issue (smaller scope = nicer to review :))

Don't worry about the PlatformOperations then, does look like we shouldn't be hitting it at all :)

RxApp.MainThreadScheduler = DefaultScheduler.Instance;
}
}
}
8 changes: 6 additions & 2 deletions src/ReactiveUI/ReactiveUI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard1.1;net45;uap10.0;Xamarin.iOS10;Xamarin.Mac20;MonoAndroid70</TargetFrameworks>
<TargetFrameworks>netstandard1.1;net45;uap10.0;Xamarin.iOS10;Xamarin.Mac20;MonoAndroid70;netcoreapp1.0</TargetFrameworks>
<AssemblyName>ReactiveUI</AssemblyName>
<RootNamespace>ReactiveUI</RootNamespace>
<Description>A MVVM framework that integrates with the Reactive Extensions for .NET to create elegant, testable User Interfaces that run on any mobile or desktop platform. Supports Xamarin.iOS, Xamarin.Android, Xamarin.Mac, Xamarin Forms, WPF, Windows Forms, Windows Phone 8.1, Windows Store and Universal Windows Platform (UWP).</Description>
Expand Down Expand Up @@ -51,7 +51,11 @@
<ItemGroup Condition=" '$(TargetFramework)' == 'MonoAndroid70' ">
<Compile Include="Platforms\android\**\*.cs" />
<Compile Include="Platforms\xamarin-common\**\*.cs" />
</ItemGroup>
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">
<Compile Include="Platforms\netcoreapp1.0\**\*.cs" />
Copy link
Member

@olevett olevett Aug 8, 2017

Choose a reason for hiding this comment

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

Related to this, I'd add an additional PropertyGroup to Directory.build.targets for netcoreapp. I think it might make sense to define the NETFX_CORE constant there too, but can't remember what exactly that includes off the top of my head. Will double check when I get the chance.

Copy link
Contributor Author

@jkoritzinsky jkoritzinsky Aug 8, 2017

Choose a reason for hiding this comment

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

I'll add the PropertyGroup, but I can't add the NETFX_CORE define since that define currently backs all usings of the Windows.UI.* namespaces.

Copy link
Member

@olevett olevett Aug 26, 2017

Choose a reason for hiding this comment

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

That seems fine then, might be we want to make NETFX_CORE two separate defines in future, but don't think it should be in scope for this change.

</ItemGroup>

<ItemGroup>
<None Update="VariadicTemplates.tt" Generator="TextTemplatingFileGenerator" LastGenOutput="VariadicTemplates.cs" />
Expand Down