Skip to content

ci: test doc generation for modules#177

Open
ungeskriptet wants to merge 7 commits into
nvmd:developfrom
ungeskriptet:treefmt
Open

ci: test doc generation for modules#177
ungeskriptet wants to merge 7 commits into
nvmd:developfrom
ungeskriptet:treefmt

Conversation

@ungeskriptet
Copy link
Copy Markdown
Contributor

@ungeskriptet ungeskriptet commented Apr 20, 2026

Having a formatter significantly improves code quality and also helps contributors maintain a consistent coding style. The code can be formatted by using the command nix fmt.

I have also added missing descriptions to options to fix nixos-rebuild when documentation.nixos.includeAllModules is enabled.

@nvmd
Copy link
Copy Markdown
Owner

nvmd commented Apr 26, 2026

Hi @ungeskriptet, thanks for your PR. I appreciate the efforts to include lots of smaller fixes into on PR, but could you please submit the separate ones for the evaluation fixes and for the formatting?
I think it makes a great sense to separate these concerns: fewer changes to review, formatting discussions won't block the evaluation fixes being accepted.

@ungeskriptet ungeskriptet changed the title Add formatter and fix evaluation errors Add formatter and fix build errors Apr 28, 2026
@ungeskriptet
Copy link
Copy Markdown
Contributor Author

Hi @ungeskriptet, thanks for your PR. I appreciate the efforts to include lots of smaller fixes into on PR, but could you please submit the separate ones for the evaluation fixes and for the formatting? I think it makes a great sense to separate these concerns: fewer changes to review, formatting discussions won't block the evaluation fixes being accepted.

Hi, this PR is pretty much already split into smaller fixes. Every commit except the last one (treewide: run nix fmt) I have made by hand. To test it yourself you can cherry-pick every commit except the last and run nix fmt and get the same result as me. If you only cherry-pick the first commit you can see the warnings that nixf-diagnose suggested to fix (mostly just unused arguments and variables).

@doronbehar
Copy link
Copy Markdown

@ungeskriptet maybe it'd be nice to add the last nixfmt commit hash to a .git-blame-ignore-revs like you have in Nixpkgs.

Comment thread modules/system/boot/loader/raspberrypi/default.nix Outdated
@doronbehar doronbehar mentioned this pull request Apr 30, 2026
@ungeskriptet ungeskriptet changed the title Add formatter and fix build errors ci: test doc generation for modules May 1, 2026
@ungeskriptet
Copy link
Copy Markdown
Contributor Author

I have update this PR to make CI check whether modules can evaluate and have their options properly defined. This should hopefully avoid missing descriptions in the future :)

To achieve this, I had to switch the CI runner to aarch64, since some of the modules will only evaluate on aarch64-linux.

When running nix flake check on an aarch64-linux system, all modules that are listed in nixosModules will be checked one bye one.

Missing descriptions can cause build errors during `nixos-rebuild` when
the following option is enabled:
```
documentation.nixos.includeAllModules = true;
```

Signed-off-by: David Wronek <david.wronek@mainlining.org>
Import dependencies to allow standalone usage.

Signed-off-by: David Wronek <david.wronek@mainlining.org>
This will help `nix flake check` import this file as a module.

Signed-off-by: David Wronek <david.wronek@mainlining.org>
Set arbitrary default for `cfg.variant` to make evaluation possible
when using this module standalone.

Signed-off-by: David Wronek <david.wronek@mainlining.org>
All items in the packages output must be derivations, otherwise
`nix flake check` will fail.

Signed-off-by: David Wronek <david.wronek@mainlining.org>
This will ensure all options have descriptions and are properly defined.

To check, use the following command:
```
nix flake check
```

Signed-off-by: David Wronek <david.wronek@mainlining.org>
Signed-off-by: David Wronek <david.wronek@mainlining.org>
Copy link
Copy Markdown

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

LGTM mostly.

Comment thread .github/workflows/ci.yaml
runs-on: ubuntu-latest
check:
name: Check flake
runs-on: ubuntu-24.04-arm
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there no ubuntu-flake-arm docker image to run on?

Copy link
Copy Markdown
Contributor Author

@ungeskriptet ungeskriptet May 2, 2026

Choose a reason for hiding this comment

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

These aren't Docker images to my knowledge. It's the underlaying host system.

Comment thread flake.nix
Comment on lines -188 to -201
# see legacyPackages.<system>.linuxAndFirmware for other versions of
# the bundle
inherit (pkgs.linuxAndFirmware.default)
linux_rpi5
linuxPackages_rpi5
linux_rpi4
linuxPackages_rpi4
linux_rpi3
linuxPackages_rpi3
linux_rpi02
linuxPackages_rpi02
raspberrypifw
raspberrypiWirelessFirmware
;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is kind of a breaking change... I wonder whether an evaluation error should be thrown instead? But maybe it will cause nix flake check failures... I'm also surprised that nix flake check doesn't allow attribute sets of packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have tried adding a throw statement here but that unfortunately also causes nix flake check to fail (since it's not a derivation). But evaluation will fail with an error anyway since the package will be missing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants