doc: add contribution guidelines section
This adds a section in the documentation for describing a list of guidelines that code in Home Manager should follow. This also updates the pull request template to reference this new section.
This commit is contained in:
parent
728c3eba67
commit
c8b73e415a
24
.github/PULL_REQUEST_TEMPLATE.md
vendored
24
.github/PULL_REQUEST_TEMPLATE.md
vendored
|
@ -1,16 +1,26 @@
|
|||
### Description
|
||||
|
||||
<!--
|
||||
|
||||
Please fill the description and checklist to the best of your
|
||||
ability.
|
||||
Please provide a brief description of your change.
|
||||
|
||||
-->
|
||||
|
||||
### Description
|
||||
|
||||
|
||||
|
||||
### Checklist
|
||||
|
||||
<!--
|
||||
|
||||
Please go through the following checklist before opening a non-WIP
|
||||
pull-request.
|
||||
|
||||
Also make sure to read the guidelines found at
|
||||
|
||||
https://github.com/rycee/home-manager/blob/master/doc/contributing.adoc#sec-guidelines
|
||||
|
||||
-->
|
||||
|
||||
- [ ] Change is backwards compatible.
|
||||
|
||||
- [ ] Code formatted with `./format`.
|
||||
|
||||
- [ ] Code tested through `nix-shell --pure tests -A run.all`.
|
||||
|
@ -25,7 +35,7 @@
|
|||
{long description}
|
||||
```
|
||||
|
||||
See [CONTRIBUTING](https://github.com/rycee/home-manager/blob/master/doc/contributing.adoc#commits) for more information and [recent commit messages](https://github.com/rycee/home-manager/commits/master) for examples.
|
||||
See [CONTRIBUTING](https://github.com/rycee/home-manager/blob/master/doc/contributing.adoc#sec-commit-style) for more information and [recent commit messages](https://github.com/rycee/home-manager/commits/master) for examples.
|
||||
|
||||
- If this PR adds a new module
|
||||
|
||||
|
|
|
@ -10,29 +10,16 @@
|
|||
:nixfmt: https://github.com/serokell/nixfmt/
|
||||
:example-commit-message: https://github.com/rycee/home-manager/commit/69f8e47e9e74c8d3d060ca22e18246b7f7d988ef
|
||||
|
||||
Contributions to Home Manager are very welcome. We provide some guidelines to make the process as smooth as possible for both you and the Home Manager developers.
|
||||
Contributions to Home Manager are very welcome. To make the process as smooth as possible for both you and the Home Manager maintainers we provide some guidelines that we ask you to follow. See <<sec-contrib-getting-started>> for information on how to set up a suitable development environment and <<sec-guidelines>> for the actual guidelines.
|
||||
|
||||
If you are only looking to report a problem then it is sufficient to read <<sec-reporting-issue>>.
|
||||
This text is mainly directed at those who would like to make code contributions to Home Manager. If you just want to report a bug then first look among the already {open-issues}[open issues], if you find one matching yours then feel free to comment on it to add any additional information you may have. If no matching issue exists then go to the {new-issue}[new issue] page and write a description of your problem. Include as much information as you can, ideally also include relevant excerpts from your Home Manager configuration.
|
||||
|
||||
If you want to directly contribute code then please also read through <<sec-contributing-code>>.
|
||||
|
||||
[[sec-reporting-issue]]
|
||||
=== Reporting an issue
|
||||
|
||||
If you notice a problem with Home Manager and want to report it then have a look among the already {open-issues}[open issues], if you find one matching yours then feel free to comment on it to add any additional information you may have.
|
||||
|
||||
If no matching issue exists then go to the {new-issue}[new issue] page and write a description of your problem. Include as much information as you can, ideally also include relevant excerpts from your Home Manager configuration.
|
||||
|
||||
[[sec-contributing-code]]
|
||||
=== Contributing code
|
||||
|
||||
If you want to contribute code to improve Home Manager then please follow these guidelines.
|
||||
|
||||
==== Fork and create a pull request
|
||||
[[sec-contrib-getting-started]]
|
||||
=== Getting started
|
||||
|
||||
If you have not previously forked Home Manager then you need to do that first. Have a look at GitHub's {fork-a-repo}[Fork a repo] for instructions on how to do this.
|
||||
|
||||
Once you have a fork of Home Manager you should create a branch starting at the most recent `master` branch. Give your branch a reasonably descriptive name. Commit your changes to this branch and when you are happy with the result push the branch to GitHub and {create-a-pull-request}[create a pull request].
|
||||
Once you have a fork of Home Manager you should create a branch starting at the most recent `master` branch. Give your branch a reasonably descriptive name. Commit your changes to this branch and when you are happy with the result and it fulfills <<sec-guidelines>> then push the branch to GitHub and {create-a-pull-request}[create a pull request].
|
||||
|
||||
Assuming your clone is at `$HOME/devel/home-manager` then you can make the `home-manager` command use it by either
|
||||
|
||||
|
@ -55,7 +42,86 @@ and running `home-manager switch` to activate the change. Afterwards, `home-mana
|
|||
|
||||
The first option is good if you only temporarily want to use your clone.
|
||||
|
||||
==== Commits
|
||||
[[sec-guidelines]]
|
||||
=== Guidelines
|
||||
:irc-home-manager: https://webchat.freenode.net/?url=irc%3A%2F%2Firc.freenode.net%2Fhome-manager
|
||||
:valuable-options: https://github.com/Infinisil/rfcs/blob/config-option/rfcs/0042-config-option.md#valuable-options
|
||||
:rfc-42: https://github.com/Infinisil/rfcs/blob/config-option/rfcs/0042-config-option.md
|
||||
|
||||
If your contribution satisfy the following rules then there is a good chance it will be merged without too much trouble. The rules are enforced by the Home Manager maintainers and to a lesser extent the Home Manager CI system.
|
||||
|
||||
If you are uncertain how these rules affect the change you would like to make then feel free to start a discussion in the {irc-home-manager}[#home-manager] IRC channel, ideally before you start developing.
|
||||
|
||||
[[sec-guidelines-back-compat]]
|
||||
==== Maintain backward compatibility
|
||||
|
||||
Your contribution should never cause another user's existing configuration to break. Home Manager is used in many different environments and you should consider how you change may effect others. For example,
|
||||
|
||||
- Does your change work for people that do not use NixOS? Consider other GNU/Linux distributions and macOS.
|
||||
- Does your change work for people whose configuration is built on one system and deployed on another system?
|
||||
|
||||
[[sec-guidelines-forward-compat]]
|
||||
==== Keep forward compatibility in mind
|
||||
|
||||
The master branch of Home Manager tracks the unstable channel of Nixpkgs, which may update package versions at any time. It is therefore important to consider how a package update may affect your code and try to reduce the risk of breakage.
|
||||
|
||||
The most effective way to reduce this risk is to follow the advice in <<sec-guidelines-valuable-options>>.
|
||||
|
||||
[[sec-guidelines-valuable-options]]
|
||||
==== Add only valuable options
|
||||
|
||||
When creating a new module it is tempting to include every option supported by the software. This is _strongly_ discouraged. Providing many options increases maintenance burden and risk of breakage considerably. This is why only the most {valuable-options}[important software options] should be modeled explicitly. Less important options should be expressible through an `extraConfig` escape hatch.
|
||||
|
||||
A good rule of thumb for the first implementation of a module is to only add explicit options for those settings that absolutely must be set for the software to function correctly. It follows that a module for software that provides sensible default values for all settings would require no explicit options at all.
|
||||
|
||||
If the software uses a structured configuration format like a JSON, YAML, INI, TOML, or even a plain list of key/value pairs then consider using a `settings` option as described in {rfc-42}[Nix RFC 42].
|
||||
|
||||
[[sec-guidelines-add-tests]]
|
||||
==== Add relevant tests
|
||||
|
||||
If at all possible, make sure to add new tests and expand existing tests so that your change will keep working in the future. See <<sec-tests>> for more information about the Home Manager test suite.
|
||||
|
||||
All contributed code _must_ pass the test suite.
|
||||
|
||||
[[sec-guidelines-module-maintainer]]
|
||||
==== Add yourself as a module maintainer
|
||||
|
||||
Every new module _must_ include a named maintainer using the `meta.maintainers` attribute. If you are a user of a module that currently lacks a maintainer then please consider adopting it.
|
||||
|
||||
If you are present in the NixOS maintainer list then you can use that entry. If you are not then you can add yourself to `modules/lib/maintainers.nix` in the Home Manager project.
|
||||
|
||||
Also add yourself to `.github/CODEOWNERS` as owner of the associated module files, including the test files. You will then be automatically added as a reviewer on any new pull request that touches your files.
|
||||
|
||||
Maintainers are encouraged to join the IRC channel and participate when they have opportunity.
|
||||
|
||||
[[sec-guidelines-code-style]]
|
||||
==== Format your code
|
||||
|
||||
Make sure your code is formatted as described in <<sec-code-style>>. To maintain consistency throughout the project you are encouraged to browse through existing code and adopt its style also in new code.
|
||||
|
||||
[[sec-guidelines-commit-message-style]]
|
||||
==== Format your commit messages
|
||||
|
||||
Similar to <<sec-guidelines-code-style>> we encourage a consistent commit message format as described in <<sec-commit-style>>.
|
||||
|
||||
[[sec-guidelines-news-style]]
|
||||
==== Format your news entries
|
||||
|
||||
If your contribution includes a change that should be communicated to users of Home Manager then you can add a news entry. The entry must be formatted as described in <<sec-news>>.
|
||||
|
||||
When new modules are added a news entry should be included but you do not need to create this entry manually. The merging maintainer will create the entry for you. This is to reduce the risk of merge conflicts.
|
||||
|
||||
[[sec-guidelines-conditional-modules]]
|
||||
==== Use conditional modules and news
|
||||
|
||||
Home Manager includes a number of modules that are only usable on some of the supported platforms. The most common example of platform specific modules are those that define systemd user services, which only works on Linux systems.
|
||||
|
||||
If you add a module that is platform specific then make sure to include a condition in the `loadModule` function call. This will make the module accessible only on systems where the condition evaluates to `true`.
|
||||
|
||||
Similarly, if you are adding a news entry then it should be shown only to users that may find it relevant, see <<sec-news>> for a description of conditional news.
|
||||
|
||||
[[sec-commit-style]]
|
||||
=== Commits
|
||||
|
||||
The commits in your pull request should be reasonably self-contained, 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.
|
||||
|
||||
|
@ -86,13 +152,15 @@ which ticks all the boxes necessary to be accepted in Home Manager.
|
|||
|
||||
Finally, when adding a new module, say `programs/foo.nix`, we use the fixed commit format `foo: add module`. You can, of course, still include a long description if you wish.
|
||||
|
||||
==== Style guidelines
|
||||
[[sec-code-style]]
|
||||
=== Code Style
|
||||
|
||||
The code in Home Manager is formatted by the {nixfmt}[nixfmt] tool and the formatting is checked in the pull request tests. Run the `format` tool inside the project repository before submitting your pull request.
|
||||
|
||||
Note, we prefer `lowerCamelCase` for variable and attribute names with the accepted exception of variables directly referencing packages in Nixpkgs which use a hyphenated style. For example, the Home Manager option `services.gpg-agent.enableSshSupport` references the `gpg-agent` package in Nixpkgs.
|
||||
|
||||
==== News
|
||||
[[sec-news]]
|
||||
=== News
|
||||
|
||||
Home Manager includes a system for presenting news to the user. When making a change you, therefore, have the option to also include an associated news entry. In general, a news entry should only be added for truly noteworthy news. For example, a bug fix or new option does generally not need a news entry.
|
||||
|
||||
|
@ -135,7 +203,8 @@ condition = hostPlatform.isLinux;
|
|||
+
|
||||
should be added. If you contribute a module then you don't need to add this entry, the merger will create an entry for you.
|
||||
|
||||
==== Tests
|
||||
[[sec-tests]]
|
||||
=== Tests
|
||||
|
||||
Home Manager includes a basic test suite and it is highly recommended to include at least one test when adding a module. Tests are typically in the form of "golden tests" where, for example, a generated configuration file is compared to a known correct file.
|
||||
|
||||
|
|
Loading…
Reference in a new issue