Switch from base64 to data-encoding

[sdroege]

It offers the same functionality and provides a simpler API that provides exactly what is needed here.

• I've included my change in CHANGELOG.md

[juntyr]

?r @torkleyy

[juntyr]

@sdroege Does data-encoding provide the same base64 encoding scheme as the base64 crate did? Could we perhaps add some tests to ensure that this would not break any parsing?

[codecov-commenter]

Codecov Report

Base: 84.52% // Head: 87.23% // Increases project coverage by +2.70% 🎉

Coverage data is based on head (903483c) compared to base (55ba26f).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #434      +/-   ##
==========================================
+ Coverage   84.52%   87.23%   +2.70%     
==========================================
  Files          59       59              
  Lines        7470     7238     -232     
==========================================
  Hits         6314     6314              
+ Misses       1156      924     -232     

Impacted Files	Coverage Δ
src/parse.rs	92.39% <ø> (+19.69%)	⬆️
src/de/mod.rs	71.71% <100.00%> (+1.14%)	⬆️
src/error.rs	36.74% <100.00%> (+3.41%)	⬆️
src/ser/mod.rs	72.29% <100.00%> (+1.61%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[sdroege]

Does data-encoding provide the same base64 encoding scheme as the base64 crate did?

Yes, and like base64 it also provides a few others under different names.

Could we perhaps add some tests to ensure that this would not break any parsing?

Sure, anything specific you're thinking of here or just a few encode/decode tests that contain base64 encoded bytes?

[juntyr]

Does data-encoding provide the same base64 encoding scheme as the base64 crate did?

Yes, and like base64 it also provides a few others under different names.

Could we perhaps add some tests to ensure that this would not break any parsing?

Sure, anything specific you're thinking of here or just a few encode/decode tests that contain base64 encoded bytes?

I think one roundtrip test for all three places where base64 is used should be good + one to ensure we are really using the standard base64 encoding with padding (i.e. the equivalent of https://docs.rs/base64/latest/base64/engine/general_purpose/constant.STANDARD.html)

[sdroege]

using the standard base64 encoding with padding

Yeah that's exactly it :)

I'll add some tests later!

[juntyr]

LGTM, I'll give @torkleyy a few days to give a review as well, otherwise I'll merge :)

[juntyr]

LGTM

[a1phyr]

As a RON user, I would prefer if ron sticked to use base64. It is a more widely used crate, and is far more likely to already be part of a dependency tree. A new dependency is not worth a few cosmetic change in this codebase IMO (I am not the one who maintains it, though).

[juntyr]

As a RON user, I would prefer if ron sticked to use base64. It is a more widely used crate, and is far more likely to already be part of a dependency tree. A new dependency is not worth a few cosmetic change in this codebase IMO (I am not the one who maintains it, though).

Thank you for voicing your concerns. I am pretty personally quite neural about which of the two libraries are used. base64 has a larger user base, though data-encoding is no small library either. I'm open to listening to more feedback on this change and postpone merging until then.

[juntyr]

Since RON will most likely be moving away from base64 (see #438), I will close this PR for now. Thank you @sdroege for your contribution!