Skip to content

Comments

lib.oldestSupportedReleaseIsAtLeast: rename from bad name#347258

Merged
infinisil merged 2 commits intoNixOS:masterfrom
roberth:lib-oldestSupportedReleaseIsAtLeast
Oct 9, 2024
Merged

lib.oldestSupportedReleaseIsAtLeast: rename from bad name#347258
infinisil merged 2 commits intoNixOS:masterfrom
roberth:lib-oldestSupportedReleaseIsAtLeast

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.
@roberth roberth requested a review from infinisil as a code owner October 8, 2024 09:06
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Oct 8, 2024
@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: testing Tooling for automated testing of packages and modules 6.topic: module system About "NixOS" module system internals labels Oct 8, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: lib The Nixpkgs function library 6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: testing Tooling for automated testing of packages and modules 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants