Skip to content

Conversation

@nickygerritsen
Copy link
Member

This reverts part of 761f323 since Doctrine stroes decimals as strings to not loose precision.

When we upgrade to PHP 8.4, we can use a differnt type to store them as BCMath\Number objects instead, which seems to be the better way to do this.

@vmcj
Copy link
Member

vmcj commented Oct 26, 2025

Is there a reason to keep the float in this case? Why can't we just pick 2 of the 3 options?

@nickygerritsen
Copy link
Member Author

Is there a reason to keep the float in this case? Why can't we just pick 2 of the 3 options?

See slack. I tried this and it is very messy to do so since we need to change many places where these functions are called. Like 100+.

I'd rather tackle that once we can use the new Number class

Copy link
Member

@vmcj vmcj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a typo in the commit message.

@nickygerritsen
Copy link
Member Author

You have a typo in the commit message.

I think I'm blind. What's the typo?

@meisterT
Copy link
Member

stroes -> stores

This reverts part of 761f323 since Doctrine stores decimals as strings to not loose precision.

When we upgrade to PHP 8.4, we can use a differnt type to store them as `BCMath\Number` objects instead, which seems to be the better way to do this.
@nickygerritsen
Copy link
Member Author

Lol on the mobile app I didn't even see I had more than one line. Fixed.

@nickygerritsen nickygerritsen added this pull request to the merge queue Oct 27, 2025
Merged via the queue into DOMjudge:main with commit e453219 Oct 27, 2025
36 checks passed
@nickygerritsen nickygerritsen deleted the decimal-string branch October 27, 2025 07:47
@vmcj
Copy link
Member

vmcj commented Oct 27, 2025

You have a typo in the commit message.

I think I'm blind. What's the typo?

differnt

But I see you already merged because of another typo.

@nickygerritsen
Copy link
Member Author

You have a typo in the commit message.

I think I'm blind. What's the typo?

differnt

But I see you already merged because of another typo.

🫠

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.

3 participants