Modernize project and target .NET Standard 2.0#12
Modernize project and target .NET Standard 2.0#120xced wants to merge 2 commits intoemmettnicholas:masterfrom
Conversation
There were two issues in the validation. 1. Parsing the version as a double with the _current_ culture: the decimal separator is not necessarily a dot but might be a comma for example. 2. Using a double to compare a version number. Comparing with the double type, 2.11 is smaller than 2.2 which is not what we want when comparing version numbers. Using System.Version to compare the version numbers fixes both issues.
|
|
||
| public static class SortsBySortType<TSort> where TSort : struct // proxy for "where TSort: enum" | ||
| { | ||
| static SortsBySortType() |
There was a problem hiding this comment.
This is not used. How did you preserve the original logic?
& The same question for ApiFieldsByName.
There was a problem hiding this comment.
This is not used.
I don't understand what you mean, I think it's definitely used. Maybe you got fooled by all the parameters being grayed out by ReSharper? (They are greyed out because the are only used for precondition checks, which is the purpose of this ValidateSortMinMax method.)
The same question for
ApiFieldsByName.
I haven't really looked into this. I saw some manual code to map the JSON with System.Web.Script.Serialization (which I have never used) and figured that replacing all this manual mapping with System.Text.Json would be semantically equivalent. All the tests passing comforted me into this idea.
There was a problem hiding this comment.
ad SortsBySortType - please disregard my remark. I must have overlooked that
ApiFieldsByName - yeah, that seems to be the case :)
let's see what @emmettnicholas has to say regarding this PR
|
@0xced It looks good overall! :) I have just two little remarks. |
* Upgrade projects to the new "SDK style" csproj format
* Remove the packages directory and packages.config file
* Remove `AssemblyInfo.cs` files (replaced with equivalent csproj properties)
* Remove `*.vsmdi` and `*.testsettings` files
* Run tests on .NET Framework 4.7.2 + .NET Core 3.1 + .NET 6.0
* Update test dependencies to their latest version
* Microsoft.NET.Test.Sdk → 17.1.0
* Moq → 4.16.1
* MSTest.TestAdapter → 2.2.8
* MSTest.TestFramework → 2.2.8
* Use `System.Text.Json` instead of `System.Web.Script.Serialization`
* Use `JsonPropertyNameAttribute` instead of `FieldAttribute`
* Make properties `{ get; init; }` instead of `{ get; internal set; }` with the help of the `IsExternalInit` NuGet package to support .NET Standard 2.0
* Define `JsonPropertyName` attributes for all enums and use `JsonStringEnumMemberConverter` from the `Macross.Json.Extensions` NuGet package to workaround [issues in JsonStringEnumConverter](dotnet/runtime#31619)
Fixes emmettnicholas#11
0xced
force-pushed
the
netstandard2.0
branch
from
9c9a29e to
0c90f16
Compare
2 participants
Upgrade projects to the new "SDK style" csproj format
AssemblyInfo.csfiles (replaced with equivalent csproj properties)*.vsmdiand*.testsettingsfilesUse
System.Text.Jsoninstead ofSystem.Web.Script.SerializationJsonPropertyNameAttributeinstead ofFieldAttribute{ get; init; }instead of{ get; internal set; }with the help of theIsExternalInitNuGet package to support .NET Standard 2.0JsonPropertyNameattributes for all enums and useJsonStringEnumMemberConverterfrom theMacross.Json.ExtensionsNuGet package to workaround issues in JsonStringEnumConverterFixes #11
/cc @jskeet @petrsvihlik