bitwarden / passwordless-dotnet

Bitwarden Passwordless.dev .NET SDK.
https://bitwarden.com/
Apache License 2.0
36 stars 11 forks source link

Move more model classes to records #108

Closed Tyrrrz closed 9 months ago

Tyrrrz commented 9 months ago

Found a way to assign inline docs to properties -- using the <para> on the type definition.

Migrated some of our older models to use { get; init; } and/or record where possible. This is a breaking change to models related to event logging, but those have not been released yet.

codecov[bot] commented 9 months ago

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (b39384d) 63.76% compared to head (a2807c2) 63.75%.

Files Patch % Lines
src/Passwordless/Models/Credential.cs 0.00% 18 Missing :warning:
src/Passwordless/Models/ApplicationEvent.cs 0.00% 9 Missing :warning:
src/Passwordless/Models/SetAliasRequest.cs 0.00% 3 Missing :warning:
src/Passwordless/Models/GetEventLogResponse.cs 60.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #108 +/- ## ========================================== - Coverage 63.76% 63.75% -0.02% ========================================== Files 43 43 Lines 723 720 -3 Branches 58 57 -1 ========================================== - Hits 461 459 -2 Misses 253 253 + Partials 9 8 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Tyrrrz commented 9 months ago

@jrmccannon curious about your opinion since you added some of these models

jrmccannon commented 9 months ago

A lot of the default values were mainly so that if the classes were newed up, we could avoid some null reference exceptions. Since we're now enforcing the records to be created, you would have to explicitly assign null to the newed up record.

Tyrrrz commented 9 months ago

@abergs @jrmccannon do you think this is approvable or would you rather I revert something?