add enumerated property NumericType to icu_properties #7157
Conversation
m4rch3n1ng
requested review from
a team,
Manishearth,
robertbastian and
sffc
as code owners
|
|
m4rch3n1ng
force-pushed
the
numeric-type
branch
2 times, most recently
from
1eb2117 to
ae8c118
Compare
Manishearth
left a comment
Manishearth
left a comment
There was a problem hiding this comment.
Thanks!
I forgot what our bar is for new properties, I think it's mostly "people want this". I'm going to hold off on merging this until after 2.1. But the code looks about right.
sffc
left a comment
sffc
left a comment
There was a problem hiding this comment.
Nice work! Thanks for the contribution!
provider/source/src/tests/data.rs
Outdated
| ("uprops/small/Alpha.toml", include_bytes!("../../tests/data/icuexport/uprops/small/Alpha.toml").as_slice()), | ||
| ("uprops/small/Basic_Emoji.toml", include_bytes!("../../tests/data/icuexport/uprops/small/Basic_Emoji.toml").as_slice()), | ||
| ("uprops/small/bc.toml", include_bytes!("../../tests/data/icuexport/uprops/small/bc.toml").as_slice()), | ||
| ("uprops/small/nt.toml", include_bytes!("../../tests/data/icuexport/uprops/small/nt.toml").as_slice()), |
There was a problem hiding this comment.
Observation: seems to not be in alphabetical order (here and elsewhere). I think it goes between nfkdinert and Pat_Syn
There was a problem hiding this comment.
The alphabetical order was fixed here, but not elsewhere. (optional)
There was a problem hiding this comment.
oh i didn't realise that everything was sorted alphabetically oops. give me like 5 ish minutes and i can fix it.
There was a problem hiding this comment.
oh wait the definitions in properties/src/props.rs are not sorted though, right?
There was a problem hiding this comment.
ok the ffi definitions are also not sorted, so i've just updated the bakeddata parts, should be done now ... (well unless the ci disagrees)
m4rch3n1ng
force-pushed
the
numeric-type
branch
2 times, most recently
from
015ff65 to
ca21812
Compare
m4rch3n1ng
force-pushed
the
numeric-type
branch
from
ca21812 to
01c8263
Compare
4 participants
did not find an existing issue issue, but i needed (wanted) to read this property and found out it wasn't available, so here it is ...