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

Update xunit version to 2.4 and target NS2.0 #269

Merged
merged 10 commits into from Jul 21, 2018

Conversation

ViktorHofer
Copy link
Member

cc @jorive

@@ -4,7 +4,7 @@

<PropertyGroup>
<AssemblyTitle>xunit.performance.core</AssemblyTitle>
<TargetFramework>netstandard1.3</TargetFramework>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think this could stay as netstandard1.3

Copy link
Member Author

Choose a reason for hiding this comment

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

netstandard2.0 pull less dependencies in which results in a cleaner package graph.

Copy link
Member

@jorive jorive left a comment

Choose a reason for hiding this comment

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

LGTM

@jorive
Copy link
Member

jorive commented Jul 19, 2018

@ViktorHofer there are some build failures.

@@ -72,7 +72,7 @@ setlocal
cd /d %~dp0tests\simpleharness
call :dotnet_build || exit /b 1

for %%v in (netcoreapp1.0 netcoreapp1.1 netcoreapp2.0 net461) do (
for %%v in (netcoreapp2.0 net461) do (
Copy link
Member

Choose a reason for hiding this comment

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

Should we add netcoreapp2.1 testing too, and download latest tooling too: 2.1.300?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a strong opinion here but I think netcoreapp2.0 is enough for the test matrix.

@ViktorHofer
Copy link
Member Author

Builders might be though to resolve as it conflicts with the CommandLineParser package which has beta dependencies...

@jorive
Copy link
Member

jorive commented Jul 19, 2018

Can it be fixed by specifying the package version on the test projects (use System.Resources.ResourceManager 4.3.0)?

@ViktorHofer
Copy link
Member Author

I'm not sure, I don't think so as the dependency is pulled in by CommandLineParser. But I will quickly fork it and retarget it to NS2.0 and send a PR to them. Isn't much work.

@ViktorHofer
Copy link
Member Author

Port done here: commandlineparser/commandline#307

@ViktorHofer
Copy link
Member Author

Till my PR gets merged in the commandlineparser repo I uploaded the temporarily locally built package to nuget and referenced it here to unblock further work.

@jorive
Copy link
Member

jorive commented Jul 20, 2018

Is this package in myget?

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Jul 20, 2018

nuget :)

@ViktorHofer
Copy link
Member Author

@dotnet-bot test this please

@ViktorHofer
Copy link
Member Author

To be honest, I've no idea why the other two tests are failing / why xunit.runner.utility.netstandard20.dll can't be loaded...

@ViktorHofer
Copy link
Member Author

Tests failed because of xunit/xunit#1594 (comment). It seems this was never fixed. Adding netcoreapp2.0 and net461 tfms resolves the right assembly.

@jorive
Copy link
Member

jorive commented Jul 20, 2018

Yes, this is how xunit is packaged :(

@jorive jorive merged commit df2a359 into microsoft:master Jul 21, 2018
@ViktorHofer ViktorHofer deleted the UpdateVersions branch July 21, 2018 00:33
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.

None yet

2 participants