Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Comment | File | Size | Author |
---|---|---|---|
#18 | number-field-item-1839076-18.patch | 8.45 KB | Berdir |
#12 | number-field-item-1839076-7-for-testbot.patch | 22.57 KB | Berdir |
#12 | number-field-item-1839076-7.patch | 8.47 KB | Berdir |
#7 | number-field-item-1839076-7.patch | 9.9 KB | Berdir |
#1 | number-field-item-1839076-1.patch | 8.67 KB | Berdir |
Comments
Comment #1
BerdirHere's a first shot.
Used string as the value for DecimalItem. I guess we could add a specific TypedData class, not sure.
Comment #2
tstoecklerSee #1777268-3: Add a TypedData API for allowing metadata to be associated with data and following about the whole Decimal / Float debate. I don't really know much about the whole thing, but I remembered that discussion so wanted to give a pointer, because I'm pretty sure it's related at the very least.
Comment #3
BerdirThanks. So that discussion essentially said that we do not want to add a Decimal type. Which means use string here as we did not have before and still don't have a native interpretation of this in PHP...
On the other side Language isn't a native data type in PHP either :)
Comment #5
BerdirHaven't seen that one before..
Comment #6
Berdir#1: number-field-item-1839076-1.patch queued for re-testing.
Comment #7
BerdirUpdated to use static::
Comment #8
fagoPatch looks gread and has solid test coverage - good work!
-> RTBC as long as it still applies after the comments patch was committed.
Comment #9
fago#7: number-field-item-1839076-7.patch queued for re-testing.
Comment #10
fagoComment #12
BerdirNo idea why this failed to apply.
Anyway, here is a re-roll against #1839082: Implement the new entity field API for the field test field type so that I can use the new base test class. The patch for the testbot contains the changes from that other issue so he can run the tests.
Comment #14
BerdirComment #15
das-peter CreditAttribution: das-peter commentedI know this is very nitpicky, but shouldn't we ensure there's always at least one decimal digit? Otherwise this could be a candidate for a mandelbug, e.g. if somewhere the handling of decimal digits fails or vice versa.
Could be done like this:
Comment #16
Berdir#12: number-field-item-1839076-7.patch queued for re-testing.
Comment #17
Berdir@das-peter: A float without decimal places seems like a valid value. I'll make sure that it doesn't cause any issues with an explicit test run and confirm but I don't think we need to treat that specially.
Comment #18
BerdirRe-roll that uses a fixed float value that does not cause any trouble. We have to fight with enough random fails already :)
Comment #19
fagoThanks. Patch looks good and das-peter's concerns have been addressed.
Comment #20
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.