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

[WIP] Improve Code Quality #422

Merged
merged 18 commits into from
Aug 23, 2023
Merged

[WIP] Improve Code Quality #422

merged 18 commits into from
Aug 23, 2023

Conversation

iamcarbon
Copy link
Contributor

@iamcarbon iamcarbon commented Aug 21, 2023

  • Remove platform specific logic that is no longer necessary on .NET core
  • Add implicit casting to CborByteString and CborTextString to their respective types (byte[] and string)
  • Use optimized OperatingSystem classes for platform specific logic
  • Minor test cleanup
  • Improve AndroidKey error messages
  • Improve CredentialPublicKey test coverage

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2023

Codecov Report

Merging #422 (49e4c21) into master (86b6c6d) will increase coverage by 0.43%.
The diff coverage is 79.06%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
+ Coverage   75.13%   75.56%   +0.43%     
==========================================
  Files         100      100              
  Lines        2783     2779       -4     
  Branches      460      455       -5     
==========================================
+ Hits         2091     2100       +9     
+ Misses        576      560      -16     
- Partials      116      119       +3     
Files Changed Coverage Δ
Src/Fido2.Models/COSETypes.cs 66.66% <ø> (ø)
Src/Fido2/AttestationFormat/AndroidKey.cs 94.04% <0.00%> (-1.20%) ⬇️
Src/Fido2/AttestationFormat/Apple.cs 90.32% <0.00%> (-3.23%) ⬇️
Src/Fido2/AttestationFormat/AppleAppAttest.cs 68.75% <0.00%> (-2.09%) ⬇️
Src/Fido2/AttestationFormat/Tpm.cs 93.54% <0.00%> (-0.72%) ⬇️
Src/Fido2/Extensions/CryptoUtils.cs 87.09% <0.00%> (-1.08%) ⬇️
Src/Fido2/Objects/CredentialPublicKey.cs 81.81% <33.33%> (+13.13%) ⬆️
Src/Fido2/Extensions/EcCurveExtensions.cs 77.77% <85.71%> (+34.02%) ⬆️
Src/Fido2/AttestationFormat/AndroidSafetyNet.cs 96.20% <100.00%> (ø)
Src/Fido2/AttestationFormat/FidoU2f.cs 100.00% <100.00%> (+8.57%) ⬆️
... and 4 more

... and 2 files with indirect coverage changes

@iamcarbon
Copy link
Contributor Author

@abergs Ready for review

@abergs abergs self-requested a review August 23, 2023 13:48
Src/Fido2/AttestationFormat/FidoU2f.cs Show resolved Hide resolved
Src/Fido2/Extensions/CryptoUtils.cs Outdated Show resolved Hide resolved
Test/Extensions/CertInfoHelper.cs Outdated Show resolved Hide resolved
Test/Extensions/PubAreaHelper.cs Outdated Show resolved Hide resolved
@abergs
Copy link
Collaborator

abergs commented Aug 23, 2023

@iamcarbon I understood your making some small changes? I'm OK either way.

@iamcarbon
Copy link
Contributor Author

@abergs All set. I know these small changes can get tedious to review -- and I appreciate you for for taking the time and for the feedback.

The 4.0 release is shaping up nicely!

@abergs abergs merged commit 2b0fe71 into passwordless-lib:master Aug 23, 2023
7 checks passed
@abergs
Copy link
Collaborator

abergs commented Aug 23, 2023

@iamcarbon Your contributions are highly valued!

@aseigler
Copy link
Collaborator

@iamcarbon Your contributions are highly valued!

^ Extremely!

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.

4 participants