docs: clarify module contribution guidance

Document reviewer expectations for new modules so contributors can align option design, news, test organization, and commit structure before review.

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

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.

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: