paireks / dotbim

Minimalist file format for BIM
MIT License
161 stars 20 forks source link

Change `TargetFrameworks` to netstandard2.0 and 4.0 #13

Closed ricaun closed 1 year ago

ricaun commented 1 year ago

dotnet

test

paireks commented 1 year ago

Hey Luiz!

Thank you for your input! :) I tried to check it out, and:

Let me know if you have any ideas how to fix the tests. Merry Christmas to you! :)

ricaun commented 1 year ago

Hello Wojciech!

I'm using Visual Studio and the test project has some 'xunit.runner.visualstudio' package references, I supposed that could be the issue. I usually use NUnit to make tests and run these tests in GitHub action, xunit I never used 😢

I created a new branch with the test using NUnit, https://github.com/ricaun/dotbim/tree/nunit

Another thing I changed is the Newtonsoft.Json version to 9.* and in the current configuration the package is not gonna appear with dependencies, so the project can select any version. I work with Revit plugins and each Revit version has a specific Newtonsoft.Json version, that's the reason I changed so I know is gonna work in a plugin.

You should create a develop branch, usually is not a good idea to send directly to the master branch. 😉

And Merry Christmas! 🤘

paireks commented 1 year ago

Ok, regarding tests it turned out that they run well, the test coverage doesn't work, but during some research it seems to be an issue with dotCover itself most likely. I think I can merge it then as it is, but before that I'll probably create a new Nuget and see in other projects if everything seems fine after update. Will let you know :)

P.S. - If you have any comments regarding existing Nuget regarding what should be improved or changed - let me know, so maybe we can deal with it already :)

ricaun commented 1 year ago

Everything should work fine, the main code was not changed, another thing that could be nice to add is the documentation file. I added this line in the csproj.

  <!-- Disable DocumentationFile
  <PropertyGroup Condition="!$(Configuration.Contains('Debug'))">
    <DocumentationFile>$(OutputPath)\$(AssemblyName).xml</DocumentationFile>
  </PropertyGroup>
  -->

If you enable all the public methods and classes gonna require documentation, if not implemented the compiler gonna show some warning.

paireks commented 1 year ago

Thanks for all input! Once I will create new nuget and upload it, I will merge 1_0_1 with master :)

ricaun commented 1 year ago

Neat! Is possible to automate that as well, to run some GitHub action to generate the package, sign and send to the nuget. This action is trigger when something is marged with the master branch.