only things i would set is --line-length 79 to fully embrace pep8 compliance (not a fan of black arbitrary 88)
3 Likes
I havent caught up on all the messages on here but can we have a no-FIXME in master standard?
2 Likes
Also, yeah 
. I’m so glad we’re in agreement. Black is the only formatter in any language that I’m just overall happy with. I wish there was one like it for nix.
1 Like
Yep. No commits should be made with a FIXME unless it’s an upstream issue, and even then it’d be better to add the bug to a project board. I’ll add this tomorrow
Well I think commits should be able to have FIXME for Dev branches. If two people are working on a problem its nice to be able to commit WIP stuff. The FIXME just can’t get merged into master without either being declared intentional behavior or being properly fixed.
4 Likes
While I don’t want to cause confusion as there’s probably some differences to be had between all-repo standards vs per-repo standards. And also recommended stuff vs hard rules. I’ll let someone else draw the distinction.
I want to add one bullet under standards for Aux-original nix code (rather than ported code).
With and Rec
I think the Aux crew recognizes how painful unnecessary-recursion can be for debugging and learning. On a similar note, with and rec keywords are extremely harmful to runtime evaulation performance, static code analysis tools, and debatably human readers (not authors) of code. For example, a new reader seeing with lib; with nixpkgs; split has no idea where split came from. Most of all, code can always be rewritten without loosing functionality and without using those keywords. The keywords only provide author convenience. And I have been guilty of using them too.
But for Aux specifically, I think we need the ability to do automated refactoring. We cannot scale to the O(n) needed for +80,000 packages without automation.
For three years I have tried to do an automated refactoring of nixpkgs, only to be defeated by with and rec. I can lead the way on that tooling that lets us overhaul our ecosystems/auxpkgs structure overnight, but only if we avoid using with and rec.
7 Likes
Agreed. Just to add a little we should also try to be as specific as we can with lib functions too. lib.isBool → lib.trivial.isBool
3 Likes
I agree. It’s why on the wiki it says that there shouldn’t be FIXME’s / TODO’s in merge commits
2 Likes
Commit messages
How about Conventional Commits ? It has pretty much everything laid out and there’s extensive tooling to enforce it.
Code beauty
Auto-formatters: love 'em. Cuts down on nitpicking and enforcing personal styles a lot. Dunno if there are nix linters, but would be great to use them if they exist. Stuff like “please follow this obscure rule listed 5 links in and in some random file” sap the energy out of reviewers and reviewees. Having a tool point it out right away is bliss.
The only thing I would plead for is a discussion about line-length. If I had a voice, it would be for 100 characters as 80 leads to a lot of wrapping and is night byzantine (it was introduced because the IBM punchcard had 80 characters at least according to stackoverflow). Probably a lot of us have 16:9 or 16:10 screens which can fit more than 2 punchcards on them horizontally.
Cross platform packages
The cross platform packages in nixpkgs are often handled in a single files (example). For such cases, it could help to define a standard way of handling them for example defining (where it makes sense) that each cross platform package requires a separate file/folder structure per platform that is then merged into the final package using a helper.
For example
package
├── default.nix
└── platforms
├── bsd.nix
├── darwin
│ ├── aarch64.nix
│ ├── default.nix
│ └── x86_64.nix
└── linux.nix
3 directories, 6 files
This would make it clear what is needed to build/package something for each platform, instead of having a bunch of something = "cp $src/... $out/..." ++ lib.optional platform.darwin ''sed -i ...." ++ lib.optional platform.bsd "..." in multiple places.
bsd.nix could end up being as simple as
{ pkgs, ...}:
{
buildInputs = [pkgs.somethingsomething];
}
and merged in default.nix where it knows that platforms/bsd.nix means platform.bsd should also be added to the meta section of the derivation.
Maybe the Frenchie in me is over-engineering though 
7 Likes
A full “yes please” (as proposed) for the first 2 points.
I do like the structure for “cross platform packages”, but I don’t feel qualified enough to understand possible technical ramifications so I’ll keep neutrally observant on this one.
I agree to conventional commits since that’s what I put in the templates contributing.md anyways
2 Likes
Yep, I’m in agreement with the first 2. I’ll restructure the commit message portion to follow Conventional Commits. And for the most part Code beauty is covered under the code format page. I’ll make sure to include a link to the page under a Code Format section on the project page.
I’m not sure, but I believe “cross platform packages” is a bit of a misnomer in this context; afaict these are rather platform specific changes to the ‘package definition’. I.e., it doesn’t matter if cross compiling or not, but it does matter which target platform the build is for.
Regarding the actual proposal to keep them in different files: I will throw in that this makes it more difficult to reconstruct the full ‘package definition’ of a platform specific build, since now the content of two files needs to be combined to arrive at the final construct.
I’m not sure what the right tradeoff is, though - i.e. simplicity of the default.nix that is now without any platform specifics vs additional complexity (of the already complex?) platform specific build.
Edit: Thanks for the write-up, btw! I fully agree with the first point. Regarding line length, I have to admit that even with a 16:10 screen, I like to split my editor window - the actual width I have available for each file shrinks rather fast 
1 Like