lmammino commented 2 years ago

Fixes #26

This PR accounts for fn potentially being undefined in the certificate.

While doing this work, I realised that the same 3 previously failing tests exposed another edge case: also expiration_time in the DgcCertContainer can be undefined, so I also addressed this one.

3 previously-skipped tests are now passing! 🎉

codecov-commenter commented 2 years ago

Codecov Report

Merging #27 (f5c6071) into main (29aca02) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
- Coverage   82.18%   82.16%   -0.02%     
==========================================
  Files          10       10              
  Lines        1061     1060       -1     
==========================================
- Hits          872      871       -1     
  Misses        189      189

Impacted Files	Coverage Δ
src/test.rs	`95.45% <ø> (ø)`
src/dgc_cert.rs	`97.08% <100.00%> (ø)`
src/dgc_container.rs	`78.00% <100.00%> (-0.44%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 29aca02...f5c6071. Read the comment docs.

lmammino commented 2 years ago

Thanks for the thorough review @dodomorandi! 🙌🏽

I will address all the comments you mentioned shortly. The only one i will postpone is renaming the various fields. I think it makes a lot of sense to do that and offer a more intuitive interface but I prefer to do that as part of the doc issue (#6) or in another pr.

lmammino commented 2 years ago

Created https://github.com/rust-italia/dgc/issues/28 to track the suggestion about field names

rust-italia / dgc

Makes `fn` and `expiration_time` optional #27

Codecov Report