Formatters: contributing guidelines

Hi,

I was wondering if there’s any consensus regarding what formatter to use when contributing to nixpkgs? I find myself very frustrated on a simple PR that’s been open for two weeks where I was suggested to format my code using nixfmt and then another maintainer (with merge rights) asking me to use nixpkgs-fmt, without actually explaining why (just leaving many commit suggestions). Upon asking, I was given the “explanation” (6 days later) that using one of the formatters gives “ugly” results, that’s why I should update it (again).

Apparently there’s no guideline whatsoever and there seem to be different opinions on the topic (see Formatting rules for Nixpkgs for example).

I understand the delays in replying and reviewing PRs, after all we’re all just doing unpaid work in our free time but if all we get are pedantic commit suggestions over a formatter without any details then I’m surprised such maintainers get to decide whether a PR goes in or not, and at this point, I’d rather close my open PRs and spend my free time on something else.

I love NixOS and I can swallow my pride on this one but I think this is a terrible experience for new contributors.

What can be done about it?

PS: I refrain from linking the aforementioned PR to avoid any further conflict and finger-pointing, I think that’s unnecessary.

12 Likes

There are basic formatting rules in the manual NixOS - Nixpkgs 20.09 manual (mainly two spaces indentation). People usually recommend on of the formatters when the pull request visibly deviates from the formatting rules.

It does not really matter which formatter you choose – they produce slightly different output but both still conform the coding style. (nixfmt adds the extra constraint on line length, which causes weird break between url = and the URL string, so I prefer nixpkgs-fmt, which is less opinionated in this regard but both are really fine.)

2 Likes

I’ve recently tried contributing to nixpkgs for the first time and, while everything has gone smoothly for the most part, I’ve also run into the same issue of ambiguity around formatting requirements.

In my case, I ran nixfmt on each new package before pushing thinking that, even if it isn’t a standard tool, it at least ticks all the boxes in the manual.

However, since opening the PR I have had to go back and forth multiple times trying to address different formatting nits raised by the maintainer, seemingly to appease their opinion rather than any standard formatting guideline as I’m yet to be pointed towards a standard reference or guideline to back up their requests. I’m hesitant to start addressing nits without some standard or guideline to refer to, as this just leaves the door open for endless bike-shedding.

Recommended Solution

Choose one auto-formatting tool and stick with it as a standard.

One change that happened to the Rust ecosystem that made my life as a maintainer of many crates a whole lot easier was the introduction of rustfmt as a standard formatting tool. Now, rather than bike-shedding over aesthetics in PRs, I can just let rustfmt automatically check everything and skip the entire discussion around formatting. Contributors’ lives get a whole lot easier as they no longer have to guess - they can just run the tool before they submit their PR and everyone is happy.

Obviously not everyone is happy with every detail in the output of rustfmt, but I think the vast majority of the Rust community would agree that the benefits of automating the whole process easily outweigh whatever each of our personal issues are with the style of the tool’s output.

If the nixpkgs maintainers are not comfortable in choosing a standard formatting tool (at least for the repository itself), then I would recommend being more lenient with contributions, as it really is an off-putting experience having to guess what the maintainer wants to see.


Apart from the formatting side of things, I have a huge amount of respect for the legendary work that the nixpkgs maintainers do! The way everything is handled is really impressive for the most part and I look forward to contributing further! I only raise these suggestions in the hope of making everyone’s lives easier and the contribution process even smoother :slight_smile:

8 Likes

I agree. This happens in a lot of PRs and I can understand that it is off-putting. We should just mandate a standard formatter (probably nixpkgs-fmt). All formatters have their issues, but the best way to solve that is to pick one, and fix any issues that we encounter.

rustfmt/gofmt ended a lot of bikeshedding about formatting. We should do the same.

14 Likes

Thanks a lot for your insights, I wholeheartedly agree with you and I think it would greatly improve contributions if we pick one formatter and stick with it for better or worse.

1 Like

There are basic formatting rules in the manual NixOS - Nixpkgs 20.09 manual

I think this is where most people land, I was also aware of this page, but it’s not enough.

It does not really matter which formatter you choose

I started this discussion because it actually does matter to many Nixpkgs maintainers :grinning_face_with_smiling_eyes: . Without a clear guideline and without picking a formatting tool, this just does not work.

That would be amazing. How do we make this happen? :star_struck:

1 Like

I was also running into this issue. I used nixfmt, as it is the most strict, but nixpkgs-fmt was suggested by maintainers. I have an opinion which of the two i like more, but that doesn’t matter as any one of the formatters would make things more consistent. Consistency is better than any ones opinion.

nixpkgs-fmt is less strict, but seems to suggested by maintainers most often.

How about:

  1. Suggesting the formatter in the contribution guidelines
  2. Add a task to the pr template to run the specific formatter
  3. Make it easy to use as a oneliner like nix-review
  4. Format all files in nixpkgs
7 Likes

Maybe it’s too heavy-handed, but one option would be to write an RFC. It would have the benefit that there would be no doubt what the standard formatter is (if the RFC is accepted).

9 Likes

If I remember correctly this is the only bad thing about nix-fmt. I don’t care to much about all the other stuff but breaking a 120 character line after url = to get to a magic 80’s line length is just a not great idea.

4 Likes