Skip to content

feat: remove numtide/flake-utils#488

Open
drupol wants to merge 1 commit into
masterfrom
push-tqzpwynomwop
Open

feat: remove numtide/flake-utils#488
drupol wants to merge 1 commit into
masterfrom
push-tqzpwynomwop

Conversation

@drupol

@drupol drupol commented Dec 28, 2025

Copy link
Copy Markdown
Collaborator

Let me know what you think.

@drupol
drupol requested a review from jtojnar December 29, 2025 12:17
@drupol

drupol commented Jan 3, 2026

Copy link
Copy Markdown
Collaborator Author

@jtojnar WDYT of this?

@jtojnar jtojnar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am split on this.

On one hand, I like reducing dependencies and find the more localized transformations easier to reason about. On the other hand, we no longer share pkgs across the flake attributes.

There are some unrelated changes nested in.

Comment thread flake.nix
description = "Repository of Nix expressions for old PHP versions";

inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why reorder them rather then keep them alphabetical?

Comment thread flake.nix
};

outputs =
inputs:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not keep the named arguments?

Comment thread flake.nix
Comment on lines +54 to +56
packages = forAllSystems ({ pkgs, ... }: phpPackages pkgs);

# For each supported platform,
utils.lib.eachDefaultSystem (
system:
let
# Let’s merge the package set from Nixpkgs with our custom PHP versions.
pkgs = import nixpkgs.outPath {
config = {
allowUnfree = true;
};
inherit system;
overlays = [
self.overlays.default
];
};
in
rec {
packages = {
inherit (pkgs)
php
php56
php70
php71
php72
php73
php74
php80
php81
php82
php83
php84
;
};
checks = forAllSystems (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will evaluate Nixpkgs twice when building both packages and checks. Not sure if that is bad enough to disqualify the nicer and clearer syntax.

Comment thread flake.nix
fn:
inputs.nixpkgs.lib.genAttrs inputs.nixpkgs.lib.systems.flakeExposed (
system:
fn {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GitHub confusingly highlights fn as keyword.

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.

2 participants