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

[RFC 0166] Nix formatting, take two #166

Merged
merged 39 commits into from
Mar 5, 2024
Merged

Conversation

piegamesde
Copy link
Member

@piegamesde piegamesde commented Nov 15, 2023

This is the long-awaited rework of RFC 101, to establish an official and standard Nix formatting and formatter. In short:

  • Define the initial standard Nix format
  • Establish the Nix formatter team
  • Create the official Nix formatter implementation
  • Reformat Nixpkgs with the official Nix formatter
  • Require that any default formatting in the Nix CLI must use the official Nix formatter

Check it out

The initial standard Nix format is mostly implemented in this nixfmt PR.
There may be bugs, and it may not yet correspond entirely to this RFC, but it's fairly close.

It is packaged in Nixpkgs as nixfmt-rfc-style.

To see Nixpkgs formatted using it: https://github.com/piegamesde/nixpkgs/tree/nixfmt-2

Context

This RFC is the successor to RFC 101, which originally proposed the idea of a standard formatter. The RFC started out slow, mainly trying to pick an existing formatter, but it didn't really get off the ground. Only in January 2023 when @infinisil scheduled bi-weekly calls with the shepherd team, continuous progress was made:

  • 18 meetings were held, all meeting notes are posted in RFC 101
  • A lot of formatting discussions and tests were done, which is now condensed into this new RFC
  • @piegamesde spent a lot of time to implement the decisions in nixfmt
  • Serokell, the owners of nixfmt, agreed to transfer the nixfmt package to the NixOS organisation when the RFC is merged ❤️

With almost a year of discussions, a lot of work went into this and the RFC changed completely. It would be disingenuous to continue it as the original RFC, which is why we're opening it as a new one. All 4 original shepherds are pretty happy with this RFC as is, but we're looking forward to receiving feedback. Since original shepherds @piegamesde and @infinisil are now authors of the new RFC, we're also looking to find 1-2 new shepherds to fill the gap.

The original shepherd team will continue meeting bi-weekly on Tuesday 20:00 CET here, and is reachable on Matrix.

Signed by the original shepherd team,

Create 0101-nix-formatting.md

WIP

Go through a large part and agree on it

Co-Authored-By: @piegamesde

Update 0101-nix-formatting.md

Rework a lot of things

Update 0101-nix-formatting.md

Move around some sections

Reword the detailed section

Minor updates

Slight header changes again

Updates

Update 0101-nix-formatting.md

Update after today's meeting

Update 0101-nix-formatting.md

Further updates in the meeting

Update 0101-nix-formatting.md

After todays meeting

Update after meeting

Rename to probably the right number

Only use anchor links

Improvements and additions

- The sub-expression rule is now reworded and its own section with
  examples and rationale
- Line length limit is now specified as we agreed-upon in the meeting
- The operator section is rewritten to align more with the consensus

Redo and explain operator special case

Also remove the special case for non-chainable operators, barely any benefit
in Nixpkgs
||
some function call
&& cond3;
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely stellar work! ❤️❤️❤️ Thanks so much for pulling through. I fully subscribe to the motivation and the full extent of the proposal. Once implemented, it will surely eliminate a significant source of friction for contributors and maintainers.

Great piece of writing, and a beautifully painted bike shed.

@infinisil
Copy link
Member

PSA: If you want to correct an obvious typo of an RFC, please just make a PR against the RFCs branch, this way we can limit the discussions in the PR to just the important bits ❤️

m15a added a commit to m15a/flake-fennel-tools that referenced this pull request Mar 30, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-meetup-in-darmstadt-2024-04-22/42853/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-56/43035/1

abstrnoah added a commit to abstrnoah/dotfiles that referenced this pull request Apr 12, 2024
Adopting https://github.com/NixOS/nixfmt as nix formatter for now,
mainly because
* NixOS/rfcs#166 has been accepted and it seems
  that nixfmt aims to be the primary implementation
* nixfmt is clearly being actively maintained
* nixfmt does `let` blocks (mostly) as I prefer (although I wish it
  uncomprimisingly put `in` block on new indented line.
abstrnoah added a commit to abstrnoah/dotfiles that referenced this pull request Apr 12, 2024
Adopting https://github.com/NixOS/nixfmt as nix formatter for now,
mainly because
* NixOS/rfcs#166 has been accepted and it seems
  that nixfmt aims to be the primary implementation
* nixfmt is clearly being actively maintained
* nixfmt does `let` blocks (mostly) as I prefer (although I wish it
  uncompromisingly put `in` block on new indented line).
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixf-tidy-static-semantic-linter-for-nix-help-me-to-integrate-it-with-github-actions-pr-reviews/43911/2

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