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

Upgrade projects to SDK style #383

Merged
merged 13 commits into from
Feb 28, 2021
Merged

Upgrade projects to SDK style #383

merged 13 commits into from
Feb 28, 2021

Conversation

robertcoltheart
Copy link
Contributor

@robertcoltheart robertcoltheart commented Jan 20, 2021

This PR upgrades all the projects to use the newer csproj format.

Most of the changes are:

  • Pre-processor params around .NET Framework-only code (datalink, dynamic, formatdetector etc)
  • Getting the tests working with no T4 transforms
  • Some test classes completely deleted as they weren't being included in the original csproj anyway
  • Updating and simplifying the build scripts. Everything just passes through dotnet CLI now.

One caveat is that I've dropped out the FileHelpers.Examples project from the solution, otherwise the change count on this PR would skyrocket. Added it back, though it doesn't really work, and didn't seem to work in the old version either.


namespace FileHelpers.Tests
{
public static class FileTest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class is to replace the T4 transforms. It's far from ideal, but was necessary to keep the PR file change low. This should be refactored to return strongly typed objects in the future and drop the use of dynamic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know how to fix this at the moment.
It seems the generated file is still around: GenerateTestData.autogen.cs I believe it is not of any use any more, isn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have seen that now there are 689 tests with .Net Framework. Before your change there were only 591 tests.
Do you know why that is?

@mcavigelli mcavigelli self-requested a review January 21, 2021 19:45
@mcavigelli
Copy link
Collaborator

Hi Robert

Thank you for your pullrequest.
I will look into it next weekend.

Matthias

Copy link
Collaborator

@mcavigelli mcavigelli left a comment

Choose a reason for hiding this comment

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

Unfortunately it does not work yet on my machine.
I get compilation errors, saying that global::System.Runtime.Versioning.TargetFrameworkAttribute is declared twice.

I will continue this week.

Matthias

FileHelpers.Tests/FileHelpers.Tests.csproj Outdated Show resolved Hide resolved
FileHelpers/FileHelpers.csproj Outdated Show resolved Hide resolved
FileHelpers.Tests/FileTestBase.cs Outdated Show resolved Hide resolved
@robertcoltheart
Copy link
Contributor Author

@mcavigelli My guess is that you need to clear your bin and obj folders for the compilation to work? I double checked and the build definitely works on my machine as well as on the build server.

@mcavigelli
Copy link
Collaborator

After cleaning with git clean -dxf and the corrected framework it compiles within no time!
Thank you.

Looking at the created artifacts by appveyor
https://ci.appveyor.com/project/MarcosMeli/filehelpers/builds/37541720/artifacts
it seems that

  • FileHelpers netstandard and FileHelpers.Excel do not contain the xml documentation file
  • FileHelpers.Excel nuget information is not based on the information in the git repository. Therefore the version number is not correct and other relevant information is not included. But in FileHelpers nuget package it is correctly included.

@mcavigelli
Copy link
Collaborator

This looks really like a lot of work you have done for the project!

Thanks a lot Robert.

@mcavigelli
Copy link
Collaborator

Regarding the Examples project I am thinking how to rewrite / refactor it with standard means,
ie run it with NUnit, document it with markdown files and building the webpages with docfx.

@robertcoltheart
Copy link
Contributor Author

@MarcosMeli Fixed all of the above. I modified the Appveyor script to use GitVersion which calculates the SemVer from the git repo tags. So that when you create a release in GitHub, the tag should build on Appveyor and will produce nuget packages that have the tag version. There's probably a way to do it using Appveyor itself, but I don't know Appveyor very well.

@mcavigelli mcavigelli merged commit 21c4c99 into MarcosMeli:master Feb 28, 2021
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