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

lib.oldestSupportedReleaseIsAtLeast: rename from bad name #347258

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

roberth
Copy link
Member

@roberth roberth commented Oct 8, 2024

…and deprecate after 24.11

It seems impossible to describe this condition more concisely without making it incomprehensible.

I've considered removing the old end-of-life gates, but they're still useful as "comments" that describe when an alias was deprecated, and we could instead remove the aliases altogether. We could set a policy for that, but I'd like to do one thing at a time.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

…te after 24.11

It seems impossible to describe this condition more concisely without making
it incomprehensible.
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Oct 8, 2024
Comment on lines +2140 to 2141
cartesianProductOfSets = warnIf (oldestSupportedReleaseIsAtLeast 2405)
"lib.cartesianProductOfSets is a deprecated alias of lib.cartesianProduct." cartesianProduct;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is maybe a bit too long, but I think it's a much better description:

Suggested change
cartesianProductOfSets = warnIf (oldestSupportedReleaseIsAtLeast 2405)
"lib.cartesianProductOfSets is a deprecated alias of lib.cartesianProduct." cartesianProduct;
cartesianProductOfSets = warnIf (allSupportedReleasesSupportAlternativeOriginallyIntroducedIn 2405)
"lib.cartesianProductOfSets is a deprecated alias of lib.cartesianProduct." cartesianProduct;

Hard to remove parts without distorting the meaning though :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that this description includes the policy, whereas mine only describes the mechanism. I think that's a sensible rule of thumb.

The policy should be documented, certainly, but maybe not in the name of the function.

Also, more high level level functions are possible, such as

foo =
  deprecateForAlternativeIn
    2405
    "use bar instead of foo"
    bar;
# or similar to `mkRenamedOptionModule` iirc, though could be misread
foo = deprecate { old = "package foo"; new = "bar"; sinceRelease 2405; } bar;

Also, such a function could both warn and throw if we decide a standard time frame for that.

Anyway, before veering too far off topic, I think we could do with a slightly better named low level function first.
It will save a few brain cycles when developing the higher level ones.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and again reminds me of NixOS/rfcs#33 :)

@infinisil infinisil merged commit c5cea34 into NixOS:master Oct 9, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: haskell 6.topic: lib The Nixpkgs function library 6.topic: module system About "NixOS" module system internals 6.topic: nixos 6.topic: python 6.topic: testing Tooling for automated testing of packages and modules 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants