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

Upgraded to csproj system, added netstandard2.0. #37

Merged
merged 5 commits into from
Nov 2, 2018

Conversation

xivk
Copy link
Member

@xivk xivk commented Sep 27, 2018

We upgraded things to the newer csproj way of doing things, removing the obsolete xproj and project.json files.

- Fixed paths by using Path.Combine.
- Fixed comparison code to ignore returns.
- Fixed comparison code to handle https/http differences (probably better to update the source files).
@xivk
Copy link
Member Author

xivk commented Sep 27, 2018

Added some more improvements based on feedback in #22.

I also fixed the unittests to work cross-platform, they just work with dotnet test now (they are broken on non-windows systems). There are still a few failing tests related to redirects, everything else is working fine.

@asbjornu
Copy link
Member

asbjornu commented Oct 5, 2018

As .NET Core 2.0 is EOL, perhaps we should target 2.1 instead? Otherwise this looks great. Only 3 tests fail on my MacBook Pro, do you have an idea why?

@xivk
Copy link
Member Author

xivk commented Nov 2, 2018

Thanks for your feedback, I should have replied sooner (was on holiday for a few weeks). netstandard and .net core version are unrelated.

  • netstandard2.0 is not EOL yet, so I used that as an extra target on the main project, it's also the one recommended.
  • The test project is indeed netcore2.1

I have no idea what these tests still fail but most of them failed for me before I started this update. This is already an improvement. I figured someone here could help us fixing these last tests and figure out why they are failing.

@asbjornu asbjornu merged commit d601cdf into linked-data-dotnet:master Nov 2, 2018
@asbjornu
Copy link
Member

asbjornu commented Nov 2, 2018

I agree this is an improvement regardless of the failing tests. Thanks!

@asbjornu asbjornu mentioned this pull request Jan 10, 2019
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.

2 participants