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

treewide/nixos: remove with lib; part 2 #335618

Merged
merged 142 commits into from
Aug 30, 2024

Conversation

Stunkymonkey
Copy link
Contributor

@Stunkymonkey Stunkymonkey commented Aug 18, 2024

Description of changes

part of #208242

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.

@Stunkymonkey Stunkymonkey force-pushed the treewide-nixos-remove-with-lib-2 branch 2 times, most recently from 54373e5 to 25f4ce3 Compare August 18, 2024 16:37
@philiptaron philiptaron self-requested a review August 19, 2024 19:34
@Stunkymonkey Stunkymonkey force-pushed the treewide-nixos-remove-with-lib-2 branch 10 times, most recently from 2636366 to 54d79ef Compare August 28, 2024 20:50
@Stunkymonkey Stunkymonkey marked this pull request as ready for review August 28, 2024 22:49
@Stunkymonkey Stunkymonkey requested review from mweinelt and a team as code owners August 28, 2024 22:49
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

I was only able to find a few defects in this batch.

nixos/modules/hardware/network/ath-user-regd.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/dysnomia.nix Outdated Show resolved Hide resolved
nixos/modules/services/networking/kresd.nix Outdated Show resolved Hide resolved
nixos/modules/system/boot/systemd/journald.nix Outdated Show resolved Hide resolved
@philiptaron philiptaron merged commit 9916dc8 into NixOS:master Aug 30, 2024
23 checks passed
@NickCao
Copy link
Member

NickCao commented Aug 31, 2024

This breaks the evaluation of fcitx5: #338621

@eclairevoyant
Copy link
Contributor

eclairevoyant commented Aug 31, 2024

Also lib.optionalFile is not a function that exists, how did it get caught up in this treewide? Can you post the script you used?

@azuwis azuwis mentioned this pull request Sep 3, 2024
13 tasks
@Ma27
Copy link
Member

Ma27 commented Sep 3, 2024

Also lib.optionalFile is not a function that exists, how did it get caught up in this treewide

I think the core issue here is that most of the changes just weren't tested, right?

With #339185 and #338859 I count at least three regressions.

@eclairevoyant
Copy link
Contributor

Both are issues. I'm asking for the script so we know the magnitude of the mistake.

@philiptaron
Copy link
Contributor

Both are issues. I'm asking for the script so we know the magnitude of the mistake.

I asked Felix (@Stunkymonkey) for the same thing and received this: #335603 (comment)

@eclairevoyant
Copy link
Contributor

Thanks, I'll continue the conversation on that PR then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants