Skip to content

aliases: cleanups and keep-sorted#455735

Merged
wolfgangwalther merged 6 commits intoNixOS:masterfrom
qweered:aliases-keep-sorted
Oct 26, 2025
Merged

aliases: cleanups and keep-sorted#455735
wolfgangwalther merged 6 commits intoNixOS:masterfrom
qweered:aliases-keep-sorted

Conversation

@qweered
Copy link
Contributor

@qweered qweered commented Oct 26, 2025

  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 26, 2025
@qweered qweered force-pushed the aliases-keep-sorted branch from dbdccbc to 8b0c7bf Compare October 26, 2025 07:16
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 26, 2025
@qweered qweered force-pushed the aliases-keep-sorted branch from 8b0c7bf to f54e847 Compare October 26, 2025 07:32
@qweered qweered requested a review from emilazy October 26, 2025 08:15
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Left a few comments, the pattern should be clear: Let's move all manual modifications into commits before the keep-sorted one, so that we can easily recreate the latter.

@qweered qweered changed the title aliases: keep-sorted aliases: cleanups and keep-sorted Oct 26, 2025
@qweered qweered force-pushed the aliases-keep-sorted branch 2 times, most recently from 9cd32ad to 5200c5e Compare October 26, 2025 09:29
@qweered
Copy link
Contributor Author

qweered commented Oct 26, 2025

Done

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Overall neutral on this. Some of the organization this disrupts is intentional / something I’ve found useful, but OTOH there are a lot of arbitrary weirdnesses in the file too. The aliases.nix file is somewhat unmaintainable with the current system in general, so I guess I’m really fine with any ordering :)

The general clean‐ups are definitely good, and may help with a final round of dropping with #427017.

cc @wolfgangwalther – do we want to land this? I think I’ve talked myself into it being a good idea while writing this.

@qweered
Copy link
Contributor Author

qweered commented Oct 26, 2025

Note: throw '' '' (aka multiline) need to be replaced with default throw " " for keep sorted to work

Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Good stuff, thank you!

I went through the commits, adjusted the commit messages a bit and move some more "preserve" aliases to the top in the second commit. I also dropped the commit that changed '' to " for multi-line messages, because I believe this changes the messages contents.

With the split, I was able to recreate the last commit easily, nice.

(I also adjusted the commit hash in .git-blame-ignore-revs after my rebase)

@wolfgangwalther
Copy link
Contributor

Note: throw '' need to be replaced with throw " for keep sorted to work

Oh, I didn't notice that, let me see whether I broke that with the latest drop of that commit.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 26, 2025
@wolfgangwalther
Copy link
Contributor

Note: throw '' need to be replaced with throw " for keep sorted to work

Oh, I didn't notice that, let me see whether I broke that with the latest drop of that commit.

Indeed, I see what you mean. It's odd that treefmt ran mostly fine the first time - and only when I ran it again, it started to break everything. I put that commit back in.

@mdaniels5757
Copy link
Member

Note: throw '' need to be replaced with throw " for keep sorted to work

Oh, I didn't notice that, let me see whether I broke that with the latest drop of that commit.

Indeed, I see what you mean. It's odd that treefmt ran mostly fine the first time - and only when I ran it again, it started to break everything. I put that commit back in.

Does block=yes fix this? https://github.com/google/keep-sorted?tab=readme-ov-file#blocks

@mdaniels5757
Copy link
Member

Oops it already has it :)

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 26, 2025
@wolfgangwalther
Copy link
Contributor

Does block=yes fix this? https://github.com/google/keep-sorted?tab=readme-ov-file#blocks

The problem is that block=yes works on balancing parens, quotes etc. - but has no idea of "double single quote" that we use for strings in nix. So for keep-sorted, the balancing ends with the opening quote for these strings - and then everything starts to break.

@wolfgangwalther wolfgangwalther added this pull request to the merge queue Oct 26, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 26, 2025
Merged via the queue into NixOS:master with commit 616f4cd Oct 26, 2025
25 checks passed
@qweered
Copy link
Contributor Author

qweered commented Oct 26, 2025

🎉 Same for darwin-aliases #455798

@imincik
Copy link
Contributor

imincik commented Oct 27, 2025

@qweered , @wolfgangwalther ,

it looks to me that this PR broke pkgs/top-level/aliases.nix file.

I get error: undefined variable 'xproto' error message when trying to evaluate it. Looks related to this change.

@qweered
Copy link
Contributor Author

qweered commented Oct 27, 2025

Working on a fix

@qweered qweered mentioned this pull request Oct 27, 2025
13 tasks
@qweered qweered deleted the aliases-keep-sorted branch December 3, 2025 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants