| 14 Dec 2023 |
Philip Taron (UTC-8) | * I've got Zoom if that works for you. | 03:47:06 |
@jonringer:matrix.org | Alternative to callPackage, instead allow for simple "smart" function application through callFunction :) https://github.com/NixOS/nixpkgs/pull/274179 | 08:41:58 |
nbp | In reply to @infinisil:matrix.org So we could have e.g. a module.nix file that specifies a NixOS module (or home-manager module, or some abstract module, ..) If we do so, I urge that we also think about making it discoverable, like NixOS options are today. | 10:17:12 |
Wanja Hentze | not too much time for this atm, but can perhaps fit it in tomorrow | 13:14:23 |
infinisil | In reply to @philiptaron:matrix.org Taking a look. I could pair review on a call. It's 19:45 PST and I'm available for another 45 minutes. :-) Oh yeah that was at 5am for me, maybe that won't work that well | 14:22:22 |
infinisil | In reply to @philiptaron:matrix.org Taking a look. I could pair review on a call. It's 19:45 PST and I'm available for another 45 minutes. :-) * Oh yeah that was at 5am for me, maybe that won't work that well haha | 14:22:31 |
Philip Taron (UTC-8) | In reply to @infinisil:matrix.org Oh yeah that was at 5am for me, maybe that won't work that well haha I did end up taking a look through the PR. It all looks sane. I have various code organization suggestions, but the current organization isn't bad at all and I would definitely accept it.
Most of the code organization suggestions end up being "make a method on NixpkgsProblem named for the condition that will be created", and returning Option<Self>. That would make check_values look super sleek by sequencing together all the various checks, rather than bulky due to having the checks inlined.
One "rusty" nit is to have the various things accepting &Vec<Something> accept instead the slice &[Something]. That's both lower power and more general.
Thanks for doing this work! Big win across the board already, and more to come.
| 17:18:27 |
| 15 Dec 2023 |
Philip Taron (UTC-8) | In reply to @infinisil:matrix.org I just cleaned up the PR and marked it as ready to review, it would be great if you could take a look now! Best reviewed commit-by-commit, they all come with commit messages: https://github.com/NixOS/nixpkgs/pull/272395 OK, want me to run the tests and call it a day for acceptance? | 00:11:59 |
infinisil | Philip Taron: That would be great! Just wrote a top-level comment too :) https://github.com/NixOS/nixpkgs/pull/272395#issuecomment-1857050220 | 00:12:48 |
Philip Taron (UTC-8) | Have you read this? https://qntm.org/ratchet | 00:16:49 |
infinisil | Philip Taron: No but I'll take a look now, seems relevant! | 00:18:47 |
infinisil | Done, yeah that's the general idea :D | 00:22:40 |
Philip Taron (UTC-8) | I find the name really great. At work, we write these kind of "ratchet linters" all the time. Usually just a sqlite database + some regexs. It helps having a concept for them, since they're strictly speaking not really a test (pass / fail) they're.. their own thing. | 00:23:57 |
infinisil | Ah yeah, the name is really great, I guess it would be nice to change the name of the concept in the by-name tool! | 00:25:20 |
Philip Taron (UTC-8) | Gave you a ✅ after reading through the code. | 00:26:53 |
infinisil | Btw the RFC specifies to do it in a different way, but the basic idea is the same:
All satisfying definitions that can't be automatically migrated due to the above restrictions will be added to a CI exclusion list. CI is added to ensure that all satisfying definitions except the CI exclusion list must be using the new directory structure. This means that the new directory structure becomes mandatory for new satisfying definitions after this PR. The CI exclusion list should be removed eventually once the non-automatically-migratable satisfying definitions have been manually migrated. Only in very limited circumstances is it allowed to add new entries to the CI exclusion list.
| 00:27:33 |
Philip Taron (UTC-8) | I'm all about the spirit dominating the letter. | 00:28:27 |
infinisil | I'm planning to merge https://github.com/NixOS/nixpkgs/pull/272395 once CI passes so I can move onto the follow-up PR | 16:48:29 |
Philip Taron (UTC-8) | Sounds good to me. | 16:50:25 |
| nbp changed their display name from nbp to nbp {pto}. | 19:40:55 |
| nbp changed their display name from nbp {pto} to nbp. | 19:41:16 |
problems | https://github.com/NixOS/nixpkgs/issues/273972 this seems like a good architecture issue | 21:53:58 |
| 16 Dec 2023 |
infinisil | Not sure, it's an interesting idea, but I don't think it's an overarching issue with Nixpkgs architecture | 02:31:44 |
| 19 Dec 2023 |
infinisil | Oh nice, https://github.com/NixOS/nixpkgs/pull/255023 added an unnecessary callPackage ../by-name/... { }, which won't be possible anymore after https://github.com/NixOS/nixpkgs/pull/274591, but the existing definition won't give an error! | 02:38:34 |
infinisil | That second PR is almost ready btw, after that's merged I can work on making pkgs/by-name mandatory for new packages! | 02:39:32 |
infinisil | PS: I moved the 4-weekly meeting by 2 weeks from 2023-12-26 to 2024-01-09 to avoid the holidays :) | 16:43:56 |
infinisil | This PR is ready now, reviews (by anybody) appreciated: https://github.com/NixOS/nixpkgs/pull/274591 I'd like to merge it as soon as possible, ideally this week Ping the NAT Robert Hensing (roberth), John Ericson, Growpotkin, DavHau, tomberek | 22:38:32 |
| 20 Dec 2023 |
infinisil | And I also went ahead with a draft of the next step after the above, enforcing new packages to use pkgs/by-name: https://github.com/NixOS/nixpkgs/pull/275539 | 01:17:15 |
Robert Hensing (roberth) | In reply to @infinisil:matrix.org And I also went ahead with a draft of the next step after the above, enforcing new packages to use pkgs/by-name: https://github.com/NixOS/nixpkgs/pull/275539 It says: This PR is a draft and isn't ready to be reviewed yet | 16:33:11 |
infinisil | Robert Hensing (roberth): This one is ready: https://github.com/NixOS/nixpkgs/pull/274591 | 16:34:36 |