Skip to content

Revert "treewide: drop usage of nixfmt-rfc-style alias"#433437

Merged
0x4A6F merged 2 commits intoNixOS:masterfrom
jfly:reintroduce-nixfmt-rfc-alias
Aug 13, 2025
Merged

Revert "treewide: drop usage of nixfmt-rfc-style alias"#433437
0x4A6F merged 2 commits intoNixOS:masterfrom
jfly:reintroduce-nixfmt-rfc-alias

Conversation

@jfly
Copy link
Contributor

@jfly jfly commented Aug 13, 2025

This reverts commit c19b2c3.

As per the discussion
here, it's too early to drop usage of the alias in documentation.

Removing the references is tracked by
#425583, which a member of the Nix Formatting team will do when the time is right.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

This reverts commit c19b2c3.

As per the discussion
[here](NixOS#433100 (comment)),
it's too early to drop usage of the alias in documentation.

Removing the references is tracked by
<NixOS#425583>, which a member of the
Nix Formatting team will do when the time is right.
@jfly jfly requested review from a team, MattSturgeon, SuperSandro2000 and alyssais August 13, 2025 16:57
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: kernel The Linux kernel 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: documentation This PR adds or changes documentation labels Aug 13, 2025
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

There are probably a few individual cases of documentation that could refer to "nixfmt" the project instead of nixfmt thhe package.

But those should be considered individually, and as a whole this should definitely be reverted for now.

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Aug 13, 2025

I'm not sure whether this should be reverted in full. There are some cases of docs here (where the alias was not changed), but there are also cases of update scripts - and IIRC, these have been changed already.

Edit: Congratz on that nice green checkmark, @MattSturgeon! Welcome!

@jfly
Copy link
Contributor Author

jfly commented Aug 13, 2025

There are probably a few individual cases of documentation that could refer to "nixfmt" the project instead of nixfmt hhe package.

I skimmed the diff and didn't see any situations in docs where we're clearing disambiguating between the project and the executable name. So I left these entirely reverted. But please feel free to leave comments on individual lines where you think they should change.

but there are also cases of update scripts - and IIRC, these have been changed already.

I've added a commit that drops usage of nixfmt-rfc-style alias in update scripts.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. 6.topic: kernel The Linux kernel labels Aug 13, 2025
@0x4A6F 0x4A6F merged commit 27473e9 into NixOS:master Aug 13, 2025
28 of 30 checks passed
@MattSturgeon
Copy link
Contributor

Edit: Congratz on that nice green checkmark, @MattSturgeon! Welcome!

Thanks!

there are also cases of update scripts - and IIRC, these have been changed already.

I'd argue using the alias is also relevant in nix-shell -p shebangs, as we don't know which revision of nixpkgs is on the NIX_PATH. It could easily be a stable release or an old revision of unstable; either way we need the rfc-style alias for the update script to work as expected.

(Ideally such update scripts would be using treefmt from the nixpkgs shell, but that's a wider issue than a simple revert)

@wolfgangwalther
Copy link
Contributor

(Ideally such update scripts would be using treefmt from the nixpkgs shell, but that's a wider issue than a simple revert)

Ah.. that was the change we made about update scripts. So in fact, we should not keep nixfmt in these shebangs, but also not nixfmt-rfc-style. We should remove either, similar to b68cc63.

@MattSturgeon
Copy link
Contributor

Oh yes, I remember now.

I'm still unsure if expecting users to already be in the nixpkgs shell before running update scripts is reasonable, but it is better than using the wrong formatter.

I wonder if these update scripts added back nixfmt and/or nixfmt-rfc-style to the shebang, because their respective package maintainers weren't aware they were supposed to run their update scripts from within the nixpkgs shell?

@wolfgangwalther
Copy link
Contributor

I wonder if these update scripts added back nixfmt and/or nixfmt-rfc-style to the shebang, because their respective package maintainers weren't aware they were supposed to run their update scripts from within the nixpkgs shell?

I looked into that:

@wolfgangwalther
Copy link
Contributor

Ah.. that was the change we made about update scripts. So in fact, we should not keep nixfmt in these shebangs, but also not nixfmt-rfc-style. We should remove either, similar to b68cc63.

Done in #433588.

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

Labels

6.topic: python Python is a high-level, general-purpose programming language. 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants