diff --git a/docs/manual/contributing/guidelines.md b/docs/manual/contributing/guidelines.md index a07eacf07..ecec4da9c 100644 --- a/docs/manual/contributing/guidelines.md +++ b/docs/manual/contributing/guidelines.md @@ -58,6 +58,33 @@ YAML, INI, TOML, or even a plain list of key/value pairs then consider using a `settings` option as described in [Nix RFC 42](https://github.com/NixOS/rfcs/blob/master/rfcs/0042-config-option.md). +These guidelines describe the minimum option design requirements. Before +submitting a module, compare it against the upstream documentation or +source code and verify that the generated files, services, environment +variables, and command line arguments all match the upstream behavior +you intend to expose. + +If a module installs a package, try to make the package option nullable, +for example + +``` nix +package = lib.mkPackageOption pkgs "xdg-terminal-exec" { nullable = true; }; +``` + +This lets users keep installation outside Home Manager, for example via +`apt` or because the program is built into macOS, while still using the +module for configuration. Keeping the package non-nullable is fine when +the enabled behavior structurally requires the executable or when +package-less support would make the module significantly more complex. + +Avoid generating files for empty settings, null packages, or optional +features that are not configured. + +If upstream does not use XDG paths by default but supports changing the +configuration location with an environment variable, for example +`FOO_HOME`, expose a `configDir` option and use it to respect +`home.preferXdgDirectories`. + ## Add relevant tests {#sec-guidelines-add-tests} If at all possible, make sure to add new tests and expand existing tests @@ -201,6 +228,11 @@ that is, each commit should make sense in isolation. In particular, you will be asked to amend any commit that introduces syntax errors or similar problems even if they are fixed in a later commit. +Keep commits atomic and separated by concern. For example, a new +maintainer entry should be a separate first commit, and a shared module +should be committed separately from integrations in existing modules. +Pull requests should not include merge commits or fixup commits. + The commit messages should follow the [seven rules](https://chris.beams.io/posts/git-commit/#seven-rules), except for \"Capitalize the subject line\". We also ask you to include the affected diff --git a/docs/manual/contributing/news.md b/docs/manual/contributing/news.md index f036b36ce..adb8582cf 100644 --- a/docs/manual/contributing/news.md +++ b/docs/manual/contributing/news.md @@ -64,6 +64,10 @@ but you should follow some basic guidelines: A new module is available: 'services.foo'. + Since this news is specific to the module, its condition should use + the module enable option to avoid spamming non-users of the module, + for example `condition = config.services.foo.enable;`. + If the module is platform specific, e.g., a service module using systemd, then a condition like @@ -71,5 +75,7 @@ but you should follow some basic guidelines: condition = hostPlatform.isLinux; ``` - should be added. Use the `create-news-entry` generator described - above to scaffold this entry as part of your contribution. + should be added, either by itself for platform-scoped news or in + combination with the module enable option. Use the `create-news-entry` + generator described above to scaffold this entry as part of your + contribution. diff --git a/docs/manual/contributing/tests.md b/docs/manual/contributing/tests.md index 5a1514cd3..74284350a 100644 --- a/docs/manual/contributing/tests.md +++ b/docs/manual/contributing/tests.md @@ -54,6 +54,13 @@ The `default.nix` file should list all test cases: } ``` +Prefer keeping related assertions in as few test files as practical. +Exercising several cases in one evaluation keeps the test suite cheaper +to evaluate and reduces maintenance burden. Split cases into separate +files when they need incompatible module configuration, platform +conditions, expected assertion failures, or otherwise cannot share one +evaluation. + ### Common NMT Assertions {#sec-tests-assertions} NMT provides several assertion functions: @@ -162,6 +169,12 @@ For cross-platform modules that have packages which need to be stubbed on Darwin add the package names to `tests/darwinScrublist.nix` to prevent build failures during cross-platform test runs. +On Linux, packages are automatically scrubbed by the test infrastructure, +so tests should normally use the module's default package. Use +`test.stubs` or `config.lib.test.mkStubPackage` only when the automatic +scrubbing does not model the behavior that the test needs, such as a +package with additional files or a non-default executable layout. + ## Using the tests command {#sec-tests-command} Home Manager provides a convenient `tests` command for discovering and running tests: