Rename `provider` modules to `provider_unstable` in 2.0, and DataMarker::Yokeable to DataMarker::DataStruct?

[sffc]

trafficstars

This causes confusion, e.g. #4191

We should go out of our way to be explicit about breaking semver. How about taking another step in that direction by renaming provider to provider_unstable?

[robertbastian]

I support this, but I don't think this would have helped in #4191. The problem there was that different ICU minor releases got mixed in the presence of baked data, which should not happen anymore with ~ dependencies.

[sffc]

Yes, do this.

LGTM: @sffc @robertbastian @younies (@echeran)

[robertbastian]

Actually, do we want to call them data_unstable, or data_structs_unstable? They don't define providers.

[sffc]

Radical: just unstable. icu::decimal::unstable::DecimalSymbolsV1

[sffc]

• @robertbastian The name "unstable" by itself seems overly broad
• @sffc They don't contain data. We don't use the terminology "data struct" in code anywhere else; we talk about it in documentation, but "provider" is the namespace in code.
• @robertbastian: Currently we have DataMarker::Yokeable, that should really be DataMarker::DataStruct, which would make the data struct terminology a bit more prevalent
• @sffc Or DataMarker::Struct?
• @sffc data_structs_unstable is long / a mouthful.
• @robertbastian I like putting unstable in front because it describes that the data structs are unstable, but this only works with structs
• @robertbastian We have the #[icu_provider::data_struct] macro that marks data structs

Options:

• provider_unstable
• unstable
• data_structs_unstable
• structs_unstable
• pod_unstable
• unstable_data_structs
• unstable_structs
• unstable_pod
• unstable_data

Discussion:

• @sffc OK, I'm convinced about unstable_structs along with DataMarker::Struct
• @robertbastian: I would prefer DataMarker::DataStruct, but I can live with that. DataStruct is not actually a repetition, it'd be something like AndListV1Marker::DataStruct
• @sffc OK
• @robertbastian Note also that the module contains data markers
• @sffc Yeah, and those are not unstable in the same way that the structs are unstable. So maybe just stick with provider_unstable
• @robertbastian Yeah but the markers aren't unstable so it's not nice that they are added into the same module
• @sffc The macro could generate an unstable module: component::provider::FooV1Marker and component::provider::unstable::FooV1
• @robertbastian - then recursive types inside FooV1 can't be in the unstable module, and you can only define one marker per provider module as you can't define modules twice
• @sffc Should we just name the structs Unstable, like FooV1Unstable?
• @robertbastian Is it enough to stick with the docs? Some names are already really long
• @sffc I guess this isn't really something where we've gotten a lot of complaints from users in 1.0 when we've definitely made some breaking changes to these things

Proposal:

• Stick with provider and docs warnings
• Rename [Dynamic]DataMarker::Yokeable to [Dynamic]DataMarker::DataStruct in 2.0

LGTM: @sffc @robertbastian

[robertbastian]

Maybe we should be explicit about unstable modules and put them all behind an unstable feature. We can use instability to mark them in docs. Our internal dependencies would enable the unstable feature.

[sffc]

I'm not excited about adding features for something that isn't really a feature (doesn't enable dependencies). It doesn't serve any purpose, since the feature needs to be enabled by default, and basically everything else in the crate will need the feature to be enabled.

[robertbastian]

It does not need to be enabled by default! The feature would make the pub(crate) provider module pub, and as such it would be needed by basically just icu_provider_source.

[sffc]

Oh I see, hmm

[sffc]

What do we do with the data markers? They show up in public bounds

[robertbastian]

Unstable constructors will, of course, also be behind the unstable feature.

[robertbastian]

The unstable feature can toggle #[doc(hidden)] on the provider module and unstable constructors, this way semver checks and autocomplete will work correctly.

[Manishearth]

For UnstableSealed traits we should mark UnstableSealed and the methods of UnstableSealed traits (but not the assoc types) as cfg(not(feature = unstable), doc(hidden))