Skip to content

Conversation

@cakebaker
Copy link
Contributor

This PR bumps bigdecimal from 0.4.7 to 0.4.8 and fixes a failing test.

@RenjiSann
Copy link
Collaborator

@drinkcat If you want to take a look at the test fix

@drinkcat
Copy link
Collaborator

drinkcat commented Apr 1, 2025

Thanks for tagging me! ,-)

Huh... 3 things:

  1. The Display trait implementation is actually dead code, nothing uses it (apart from the unit test just below)...
  2. When I was looking at it, I never really quite understood what was the intent of this line anyway: Self::BigDecimal(BigDecimal::new(n * 10, 1)).fmt(f), seems like an attempt to get a decimal point in all cases? But is this the right thing to do?
  3. The failing test is inconsistent anyway: ::zero gets formatted as 0.0, but MinusZero as -0?

So... up to you. IMHO you could delete the implementation, or change the test to check for 0 instead.

If we keep the implementation, I'd remove that n*10,1 thing too (and just do Self::BigDecimal(x) => x.fmt(f)).

Oh, and a 4th thing ,-P You could grep for https://github.com/akubera/bigdecimal-rs/issues/144 in the code and remove the workaround and enable the tests as part of this (but I'm happy to do it if you don't have cycles!).

of ExtendedBigDecimal
@cakebaker cakebaker force-pushed the bump_bigdecimal branch 2 times, most recently from f6780eb to 16f75bd Compare April 1, 2025 15:27
@cakebaker
Copy link
Contributor Author

cakebaker commented Apr 1, 2025

Changes since last push:

  • removed Display implementation of ExtendedBigDecimal
  • fixed TODOs related to https://github.com/akubera/bigdecimal-rs/issues/144 (thanks for the hint about them, @drinkcat)

@github-actions
Copy link

github-actions bot commented Apr 1, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

@drinkcat
Copy link
Collaborator

drinkcat commented Apr 1, 2025

LGTM thanks!

@sylvestre sylvestre merged commit cd4d75b into uutils:main Apr 1, 2025
68 of 70 checks passed
@cakebaker cakebaker deleted the bump_bigdecimal branch April 2, 2025 05:09
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