Skip to content

refactor: remove redundant ICU version-detection mechanism#364

Open
clydegerber wants to merge 1 commit into
google:mainfrom
clydegerber:refactor/remove-redundant-version-gates
Open

refactor: remove redundant ICU version-detection mechanism#364
clydegerber wants to merge 1 commit into
google:mainfrom
clydegerber:refactor/remove-redundant-version-gates

Conversation

@clydegerber
Copy link
Copy Markdown
Contributor

Summary

The workspace's minimum-supported ICU version is 74 (per the CI matrix: 74, 76, 77), so the icu_version_64_plus, icu_version_67_plus, and icu_version_68_plus gates are always active in practice. This PR removes them entirely along with the supporting infrastructure.

Removes:

  • Redundant #[cfg(feature = \"icu_version_##_plus\")] annotations and their companion not(...) dead-code branches from rust_icu_ecma402, rust_icu_ulistformatter, rust_icu_unumberformatter, and rust_icu_uloc, including the pre-ICU-67 buggy-behavior test variant.
  • The icu_version_64_plus, icu_version_67_plus, icu_version_68_plus, and icu_version_69_max Cargo feature declarations across 27 manifests. (icu_version_69_max was already a ghost — declared widely but never consumed by any source code.)
  • The cfg-emission blocks in rust_icu_sys/build.rs and rust_icu_release/src/lib.rs.
  • The _plus features from the test-with-features CI matrix and the macos-test Makefile target.

Rewrites the CONTRIBUTING.md section as forward-looking guidance for re-introducing version-dependent code if it becomes necessary again.

Alternative approach considered

An alternative would be to keep the version-gated code intact and add build.rs files to the few crates that lack auto-detection of the ICU version (rust_icu_ecma402, rust_icu_ulistformatter, rust_icu_unumberformatter). That would preserve the public Cargo feature surface but leaves dead conditional-compilation in place — every supported ICU version satisfies the gates, so the not(...) branches are unreachable and the _plus features are activated 100% of the time. Removing the mechanism outright eliminates the dead code and simplifies the build graph; if a future ICU version ever introduces a real boundary, the pattern can be re-introduced (see CONTRIBUTING.md).

Breaking change

This removes Cargo features from the public surface of every wrapper crate. The next release already requires a major version bump for unrelated reasons (the recent PartialEq derive removals from bindgen-generated types and the Text::try_from ownership-behavior change), so this fits the breaking-changes batch.

Test plan

  • cargo test passes on default features (217 tests)
  • CI test-with-features matrix passes with the updated feature list

This commit was created by an automated coding assistant, with human supervision.

Every ICU version this workspace targets in CI (74, 76, 77) is >= the
boundary versions referenced by the version-gated cfg features
(`icu_version_64_plus`, `icu_version_67_plus`, `icu_version_68_plus`).
The gates are always active in practice, so the conditional-compilation
mechanism — gates in source code, feature declarations in Cargo.toml,
and cfg emissions from `build.rs` — is dead infrastructure.

Removes:
- Redundant `#[cfg(feature = "icu_version_##_plus")]` annotations and
  their companion `not(...)` dead-code branches from rust_icu_ecma402,
  rust_icu_ulistformatter, rust_icu_unumberformatter, and rust_icu_uloc
  (including the pre-ICU-67 buggy-behavior test variant).
- The `icu_version_64_plus`, `icu_version_67_plus`, `icu_version_68_plus`,
  and `icu_version_69_max` Cargo feature declarations across 27 manifests.
- The cfg-emission blocks in `rust_icu_sys/build.rs` and
  `rust_icu_release/src/lib.rs`.
- The `_plus` features from the `test-with-features` CI matrix and the
  `macos-test` Makefile target.

This is a breaking change to the public Cargo feature surface; the next
release will already require a major version bump for other reasons.

Rewrites the CONTRIBUTING.md section as forward-looking guidance for
re-introducing version-dependent code if it becomes necessary.

This commit was created by an automated coding assistant, with human
supervision.
@clydegerber
Copy link
Copy Markdown
Contributor Author

This is the alternative to PR 363.

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.

1 participant