Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE YET] Remove feature "randomized capitalization" #207

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

marc-vanderwal
Copy link
Contributor

Do not merge this PR yet. After #206 is merged and v2024.2 is released, the branch corresponding to this PR is to be rebased on develop. Only then can it be merged.

Purpose

This PR removes an experimental and previously deprecated “randomized capitalization” feature from Zonemaster-LDNS.

Context

See #160 for the issue and discussion, and #206 for the initial deprecation warnings.

Changes

Remove the “randomized capitalization” feature from Zonemaster-LDNS. Update the documentation accordingly.

How to test this PR

In the root of the source tree of Zonemaster::LDNS, run cpanm -v --configure-args="--random" .

Among the roughly 30 first lines of output, expect this output (the version number of Zonemaster-LDNS may vary):

Configuring Zonemaster-LDNS-4.0.2 ... Unknown option: random

Also, expect unit tests (which have been updated) to pass.

This feature has always been experimental and seems to have been
implemented a long time ago to test Zonemaster::LDNS internals. However,
there are better ways to ensure that.

This commit introduces deprecation warnings in three places:

 * in the documentation;
 * when executing Makefile.PL;
 * and during compilation of the C and XS code.

Hopefully, having the same warning in three different places will be
enough to alert end users.
Case in point (heh) of how little use the experimental case
randomization feature had: one unit tests broke without anyone noticing
it.

We can reverse this patch later when the feature is actually removed.
No longer document the --randomize command-line argument to Makefile.PL,
which is about to be removed.
Remove the possibility to define RANDOMIZE in CFLAGS by removing
--randomize as a valid CLI argument.
Remove the deprecated feature entirely.
Remove a case-insensitive comparison that was done in t/rr.t in order to
make unit tests pass on installations where the now removed case
randomization feature was enabled.

Previously, this equality comparison was done case-sensitively, which
only worked when this feature was disabled.

With this change, we can now be almost certain that we don’t do any case
alterations anymore.
@marc-vanderwal marc-vanderwal added this to the v2025.1 milestone Sep 2, 2024
@tgreenx tgreenx linked an issue Sep 12, 2024 that may be closed by this pull request
@tgreenx tgreenx added the V-Minor Versioning: The change gives an update of minor in version. label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V-Minor Versioning: The change gives an update of minor in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove feature "Randomized capitalization"
2 participants