Closed zyofeng closed 3 years ago
Thanks @zyofeng, and sorry for the delay in responding to you.
I find it a bit hard to see the project diff with all the indentation changes.
Compatibility is important. As is having no external dependencies (it's a small library). You've removed targeting net40, but it looks like there needs to be work on this to add conditional references between net40 and netcoreapp3.1, as well as tests that prove the conversion behaves.
Do you want to make some changes to the PR?
Here's what this PR needs:
<TargetFrameworks/>
.JsonConverter
(since System.Text.Json is built-in to 3.1 i.e. FrameworkReference).ShortGuidJsonConverter
has a redundant clause: reader.GetString() ?? new ShortGuid(reader.GetString())
. This should probably use an explicit cast (for clarity) e.g. (ShortGuid)reader.GetString()
which will return ShortGuid.Empty
for null or empty strings.ShortGuidTypeConverter
should also allow for Guid
conversion.If you're like me to take it over, this is the list I'll be working through.
Feel free to take over, although net40 has long gone EOL
I have added implementation for Json/TypeConverters which can be used for deserialization/model binding.