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

.NET Core 3 Port #7

Merged
merged 26 commits into from
Nov 5, 2020
Merged

.NET Core 3 Port #7

merged 26 commits into from
Nov 5, 2020

Conversation

FirehawkV21
Copy link
Contributor

Fixes #5 .

At the moment, the new projects have a copy of the code (to avoid bringing braking changes to the original) but if there are no breaking changes, it should be possible to simply edit the projects to pull the files from the originals.

@augustoproiete augustoproiete self-assigned this Aug 15, 2020
@augustoproiete augustoproiete changed the base branch from master to develop October 29, 2020 04:18
@augustoproiete
Copy link
Member

@acemod13 Thanks for updating the PR. The changes look a lot more manageable now. I'll give you some feedback throughout this week and hoping to get this merged soon.

@FirehawkV21
Copy link
Contributor Author

Aye. Looking forward to it.

I've also done some editing in the project files so it can compile both .NET 4.5 and .NET Core 3.1 libraries (mainly since the sample app wouldn't compile unless I re-wrote the entire project file).

<file src="bin\$configuration$\Ookii.Dialogs.Wpf.dll" target="lib\net45\" />
<file src="bin\$configuration$\Ookii.Dialogs.Wpf.pdb" target="lib\net45\" />
<file src="bin\$configuration$\Ookii.Dialogs.Wpf.xml" target="lib\net45\" />
<file src="bin\$configuration$\net45\Ookii.Dialogs.Wpf.dll" target="lib\net45\" />
Copy link
Member

Choose a reason for hiding this comment

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

This looks good assuming we'll continue to create the NuGet package using the NuGet CLI. I'm just wondering if we shouldn't move to dotnet pack on the .csproj now that we are going to the new SDK project format. What do you think?

<Pack>True</Pack>
<PackagePath></PackagePath>
</None>
<None Include="..\..\LICENSE">
Copy link
Member

Choose a reason for hiding this comment

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

This relates to my other question on NuGet CLI w/ .nuspec vs. dotnet pack w/ .csproj - if we are going with the latter, then let's use PackageLicenseExpression with a valid SPDX license identifier here instead of embedding a custom license file

<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
<RootNamespace>Ookii.Dialogs.Wpf</RootNamespace>
<TargetFrameworks>netcoreapp3.1;net45</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

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

Does this work fine for you with VS 2019 in this order? I remember having issues with this in some projects where it was necessary to declare net45 first, and netcoreapp3.1 after. i.e.

<TargetFrameworks>net45;netcoreapp3.1</TargetFrameworks>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works on my side. Although, I have .NET Core 3.1 SDK already installed. It is possible to specify which version to compile in Visual Studio. But, as a failsafe, it would be a good idea.

//[assembly: NeutralResourcesLanguage("en-US", UltimateResourceFallbackLocation.Satellite)]


[assembly: ThemeInfo(
Copy link
Member

Choose a reason for hiding this comment

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

Is ThemeInfo being automatically generated now through the new SDK project?

[assembly: AssemblyCopyright("Copyright (c) Sven Groot (Ookii.org) 2009")]
[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]
[assembly: System.CLSCompliant(true)]
Copy link
Member

Choose a reason for hiding this comment

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

Is CLSCompliant being automatically generated now through the new SDK project?

[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]
[assembly: System.CLSCompliant(true)]
[assembly: System.Resources.NeutralResourcesLanguage("en-US")]
Copy link
Member

Choose a reason for hiding this comment

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

Is NeutralResourcesLanguage being automatically generated now through the new SDK project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Pretty much everything that was covered under the AssemblyInfo.cs is generated through the new SDK project.

@augustoproiete
Copy link
Member

Looks great. Thanks @acemod13 for the PR and the patience. I've just released Ookii.Dialogs.Wpf v1.2.0 and I think we can get going on the .NET Core migration now, especially with the imminent .NET 5 release.

@augustoproiete augustoproiete merged commit cb6a01e into ookii-dialogs:develop Nov 5, 2020
@augustoproiete augustoproiete added this to the 2.0.0 milestone Nov 5, 2020
@augustoproiete augustoproiete mentioned this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Need .Net Core 3.0 Prerelease Version
2 participants