Gotta say, I do detest auto-formatters and the general obsessive-compulsive direction nixpkgs checks have been taking lately. This sort of linting has always been a sign to me of a project that has lost its focus and descended into navel gazing, and it feels a lot like the people who think uniformity is important imposing their will over those who realize it’s not. Like, if you create a “formatting working group”, you are inevitably going to end up enforcing a formatting style - it’s a self-selecting group, it’s the only possible outcome.
So to me, this is just yet another piece of software in my life I’ve got to fight against. It’s 100% stop-energy for a contribution.
I don’t think the point is uniformity or imposing opinions, but rather avoiding endless discussions on PRs about formatting. With a formatter you can just say “The formatter agrees with the current formatting, so this is fine to be merged.” which instead streamlines the PR process.
Review comments about formatting should just be immediately dismissed except for egregious examples. I can’t even remember the last time I saw such an example though. Nitpicking is the problem, not the solution here.
On the other hand I can point at thread upon thread and meeting upon meeting talking about formatting around the auto-formatter. However people choose to use their time is up to them, but it’s a different thing when people push their compulsions onto those with a relaxed/healthy attitude to things like formatting.
If I spend a weekend of my own time chasing down an extremely difficult bug, or perhaps bootstrapping a system repeatedly (at my own compute expense) to test a toolchain fix, only to be told by some bot that my contribution isn’t acceptable because someone with some opinion thinks I indented a line wrong, it screams mislaid priorities.
@ris This is not the time to be debating whether formatting should be enforced in Nixpkgs. That decision has been made with overwhelmingly positive support with RFC 166.
Now’s the time to improve the formatter within the established standard so that we’re mostly happy with it and can move on. So, if you have any concrete suggestions, feel free to voice them, ideally in the issue tracker, but even better with a PR.
What qualifies as an “egregious example” is certainly an opinion too
That’s why we save the reviewer round-trip and let CI highlight it, so the contributor can spend the time (what, 5 seconds?) to run nixfmt (whatever) and we never have to discuss it in the PR. It’s no longer a nitpick, but a binary “did you format” vs “did you not”.
I partially agree. Especially for new contributors, who may just be figuring out the syntax and idioms of the language for a small contribution, such “blocking” bot feedback can be disruptive and discouraging.
On the other side, accepting and accumulating individual formatting all across the codebase gets rapidly inconsistent, and then fixes to reformat it create more churn and merge conflicts for branches that may have happened in the meantime (including release backports).
The crucial thing is to recognise that these are two different problems, with different solutions (that are entangled at the point of entry).
The first is about communication and messaging for contributions. It’s possible to minimise the bad reactions, through both providing good tools in a standard devshell up front, and through choosing how variances are handled: by rejecting and complaining that the PR fails some tests, or by automating a reformatting step into the PR workflow. Oh, and of course, by having all the existing code already consistent in style so that’s what looks normal.
In order for that to work, we need good tools and they need to implement standards that are widely acceptable. As we’re seeing here, even a rough consensus is a high bar and a lot of detail work. We’re not there yet.
Eventually, existing code and new contributions can be reformatted with less friction, and without trading off against the problem that the “helpful” automatic formatting made something worse.
That’s exactly the point of an automated formatter. You just run it and can stop thinking about it. Ideally even have your editor run it automatically for you.
Also consider that not having a formatting standard does not make the opinions on formatting go away, they just aren’t standardised. Instead of one standard, you have 1000 different opinions on how code should be formatted. Each code owner would need to explain their preferred style to contributors manually and those would then need to adjust their code manually too.
I find that to be much worse because, with a standard formatter, all you need to do is run a standard tool on your files which really isn’t a lot to ask for.
It’s gotten somewhat better lately, but the amount of formatting nits i got when i started out myself was quite high, to the point where i considered it obvious to mirror that behavior when i started to review others. Thankfully i wised up, but the the amount of stalled PRs i find because of nits is still nontrivial.
I’m glad that’s been your experience, but from personal experience, I think many people here would be astonished to learn that you think formatting nitpicks in Nixpkgs were a rarity before the formatting RFCs! I sadly still see many reviews that consist entirely of basically irrelevant nitpicking and no substantive feedback today. (And when you’re not a seasoned committer and it’s not clear which reviews are going to block your PR, you don’t really have the option of being cavalier about it.)
That’s in part a social problem, of course, but it doesn’t mean it’s a net negative to apply some technology to help with it. Reducing avenues for this kind of nitpicking and establishing an automatic style that prevents the most egregious examples that could be legitimate reasons to do it are explicit stated motivations of the formatting work, from the conversations I’ve had with the team on Matrix.
Just forbidding everyone from ever commenting on formatting would be another way to achieve that goal, but I think it is one that did not seem to have consensus during the RFC discussions, and anyway eliminating the motivation people might have to do it seems better than just using an iron fist to stop them doing so.
I do actually think that formatting affects readability and the ability to easily spot things that are wrong, and that having a consistent style across the tree will be good for reasons beyond just eliminating the cycles people spend thinking about formatting. And I do actually have things I dislike about the style the formatter currently uses. But the intention is definitely to reduce those cycles for the vast majority of contributors, not increase them.
What I also like about having a single style (even if it sucks a bit right still) is that we can centralise the nitpicking and bikeshedding. It can still happen and is important to continue to happen IMHO but it would then happen in a dedicated issue tracker rather than in thousands of disparate PRs.
Again, this is a self selecting group that will only attract the interest of people who think formatting matters.
That is specifically not a solution. Pre-commit hooks are yet another piece of software to have to fight during what can be already-complicated git surgery procedures, adding extra uncertainty about what precise result is going to get committed when amending intermediate commits, and leading to merge failures when re-applying patch stacks. When you’re proposing a commit hook (or editor plugin for that matter) as a solution you are pushing your obsessions on to other people’s machines and making far too many assumptions about their workflow and tool choice, which frankly should be none of your (or nixpkgs’) business.
For remotely complex PRs, which a lot of mine are, it is also not just a matter of “just running nixfmt” when you’re done, unless you want a whole lot of PRs that have a final, semantically meaningless, “run nixfmt” commit on top of them.
Auto-formatters are the bane of my existence and I have lost literal days resolving merge conflicts arising from someone running a formatter across a code-base. In fact I would estimate that about half of the patches I’ve had to manually alter and put in-nixpkgs are solely due to people playing with formatters.
I have a hard time recognising the authority under which these enforcements are being made and see it as a special interest group doing what special interest groups so frequently do, which is find ways to force the rest of the world to march to their drum.
Is it so hard to just, y’know, leave people alone? Instead of getting your collective fingers into how they use their tools and indent their lines…
I’d say there are a lot of people with different obsessions in regards to formatting and forcing those obsessions into large projects on a per file basis is even worse than having one standard everyone is slightly unhappy about IMHO.
You can indent your lines however you want in your projects, but having a single standard is important to reduce the amount of overhead to adjust your mental parser when reading other peoples code. It is extremely tiring (at least for me) to adjust to a different coding style when reading source, which gets worse if different people touch the same files with different styles.
But as someone who doesn’t think formatting is important I would have no reason to monitor what’s going on in said RFC. Should I be monitoring all RFCs from now on in case one of them makes a decision that they’re going to do something that every single nixpkgs contributor needs to accommodate in their tooling and workflow?
Eh, that’s governance. Should you be monitoring all bills in your legislature in case one of them affects you? That’s your call. Be as involved or as uninvolved as you like. An RFC hits FCP maybe once every two months, so if you did choose to get involved in that way, that’s the time cost you’d be looking at.
If you choose not to be involved, especially on the grounds of ‘I don’t think this is important’, expect your society occasionally to require you to do things that you find inconvenient.
Currently the CI only enforces formatting if the file was already formatted prior or if it’s a new file.
So such a “run nixfmt” commit would be unnecessary and probably discouraged.
Anticipating that I would be doing a lot of work for the Darwin refactor, I went ahead and formatted the stdenv. You can see it here.
Honestly? It’s kind of ugly. Formatting the stdenv almost doubled the line count. However, since I was planning to make major changes to the bootstrap, the ugliness served as motivation to rethink how the stdenv propagates packages.
This is the current Darwin refactor stdenv bootstrap. It’s arguably much more readable than it was before, and having things grouped together in packages sets makes it easier to make sure packages are propagated correctly and (re)built in the intended stages.
I have my quibbles, but I guess that’s a way of saying I’m overall satisfied with it.
