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: Format all Nix code according to RFC-166 #232

Merged
merged 1 commit into from
Mar 31, 2024
Merged

treewide: Format all Nix code according to RFC-166 #232

merged 1 commit into from
Mar 31, 2024

Conversation

dasJ
Copy link
Member

@dasJ dasJ commented Mar 30, 2024

No description provided.

@dasJ dasJ requested review from tfc and henrik-ch March 30, 2024 09:55
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

LGTM, but I don't like one of the changes.

pills/17/config-nix.txt Outdated Show resolved Hide resolved
@drupol

This comment was marked as resolved.

@@ -1,5 +1,8 @@
{
packageOverrides = pkgs: {
graphviz = pkgs.graphviz.override { withXorg = false; };
graphviz = pkgs.graphviz.override {
# enable xorg support
Copy link
Member

Choose a reason for hiding this comment

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

This should be disable xorg support

@Ericson2314 Ericson2314 merged commit 2355217 into NixOS:master Mar 31, 2024
2 checks passed
@dasJ dasJ deleted the fix/formatting branch March 31, 2024 07:19
@henrik-ch
Copy link
Contributor

@dasJ, thank you for the comprehensive update to the repo's formatting—great work, especially given the scope! Apologies for chiming in a bit late on this.

To ensure consistency in future updates and avoid inadvertently diverging from the new standard, could you share the specific command or process you employed for this reformatting? I'm assuming there was some automation involved rather than manual adjustments?

I attempted to replicate the formatting on my end, targeting a pre-update version of the repo, using:

nix-shell -p nixpkgs-fmt --run 'nixpkgs-fmt .'

However, this only resulted in changes to a select few files within pill 20, as indicated by my git status output below:

❯ git status
HEAD detached at a66fd07
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   default.nix
	modified:   pills/20/three-hellos.nix
	modified:   pills/20/two-hellos.nix
	modified:   release.nix

no changes added to commit (use "git add" and/or "git commit -a")

Please could you share the correct approach or any insights into the process you used.

Many thanks in advance! 🙏

@dasJ
Copy link
Member Author

dasJ commented Apr 1, 2024

Unfortunately, the nix code is not actually in .nix files which meant manual work by calling the formatter manually rather than going over the entire tree :/

Also, the soon-to-be default formatter for nixpkgs will be nixfmt, not nixpkgs-fmt (I know the name is confusing). The new formatter is packaged in nixpkgs right now as nixfmt-rfc-style (not to be confused with nixfmt which has the "classic" pre-RFC style).

I know it's a mess right now but there is ongoing work to clean this up :)

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

Successfully merging this pull request may close these issues.

5 participants