Use open_enum for enumerated properties

[kupiakos]

The pattern of using an integer newtype with named associated constants is exactly what the open_enum macro aims to make cleaner. This pull should result in no semantic change. All values have stayed the same; the syntax has just changed. I've also moved some comments to docstrings.

I think it looks cleaner and I aimed to make open-enum a small dependency appropriate for icu4x - what are your thoughts?

[CLAassistant]

All committers have signed the CLA.

[Manishearth]

Personally I'm in favor of this dependency; it's a build-time only dep and it doesn't add deps other than itself. @sffc ?

[sffc]

Personally I'm in favor of this dependency; it's a build-time only dep and it doesn't add deps other than itself. @sffc ?

It's a fine dependency to add from my perspective

[jira-pull-request-webhook[bot]]

Notice: the branch changed across the force-push!

• Cargo.lock is different
• components/properties/Cargo.toml is different
• components/properties/src/props.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[kupiakos]

I just rebased against main due to a merge conflict - hopefully that's OK with your review workflow. Apparently Github handles this a lot less well than Gerrit.

[sffc]

I just rebased against main due to a merge conflict - hopefully that's OK with your review workflow. Apparently Github handles this a lot less well than Gerrit.

It's fine for smaller PRs; if there's a PR with a lot of line-specific comments, we tend to prefer merge commits over rebases, because they will get lost. We have the bot that posts when a force-push is made for better visibility.

[zbraniecki]

I don't have a strong opinion here. Trusting @nordzilla assessment.

[kupiakos]

Until "real" open enums become a part of the language, is there an advantage to this abstraction aside from readability?

No, other than me being happy to see an explicitly open enum (and my own macro) in major production code. 🙂

I'm also working on an RFC for open enums in the language (by specifying #[repr(transparent)]), but it'd be a breaking change to eventually switch to it since it is not constructed/accessed the same.

[sffc]

Discussion with @sffc @zbraniecki @Manishearth at ICU4X-SC:

ICU4X has a dependency problem; we should be very critical of taking dependencies. We can currently claim that almost all of our dependencies are ICU4X itself. All exceptions to that are dependencies that add a lot of value and are used broadly, like postcard and displaydoc. There is not consensus that open_enum as a standalone crate rises to that bar. However, we're very much looking forward to using open_enum when it gets standardized.