-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
WIP: feat(fmt): use nixfmt-rfc-style #11252
base: master
Are you sure you want to change the base?
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/formatting-team-meeting-2024-08-06/50222/2 |
if (nixfmt == "nixfmt") { | ||
try { | ||
auto res = nixpkgs.fetchTree(store); | ||
auto s = store->printStorePath(res.first) + "/ci/pinned-nixpkgs.json"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This elevates a very recent Nixpkgs implementation detail to a new Nix interface that is not consistent with anything that came before in Nix. It would be more appropriate to do this in a Nixpkgs CLI.
What if we could just load the shell instead, or just stick to the plan and finish lazy trees?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this is quite fragile. What about having a version pinned in Nix itself? A reference to nixpkgs or nixfmt repos. Benefit is thst it is simple and predictable. Downside is that you don't get an automatic upgrade.
Nixpkgs-specific tooling can do additional work to match the Nixpkgs to the fmt.
User research notes (done on Discord):
|
That all seems rather ad hoc. What if we just |
I'm thinking of the following order.
|
Isn't 3 mostly covered by the |
(ping @NixOS/nix-formatting) |
Motivation
Make
nix fmt
usable by default..#formatters.SYSTEM
orinputs.nixpkgs
is too slow for the moment, so disabling that override for now.nix develop
to obtain an ambient nixpkgsnixfmt-rfc-style
ci/pinned-nixpkgs.json
, this can cost another download+copy to store, but will be more stable over time in case someone has flake:nixpkgs set to a moving branchContext
#6087
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.