From 8feb68f9653d3ed6f371d307b1f740f14f7602a0 Mon Sep 17 00:00:00 2001 From: Peder Bergebakken Sundt Date: Thu, 26 Oct 2023 14:06:05 +0200 Subject: [PATCH] {pkgs,nixos}/README.md: Hotlink package and module reviewing guidelines, fix references --- CONTRIBUTING.md | 2 +- nixos/README.md | 13 ++++++++----- pkgs/README.md | 30 ++++++++++++++++-------------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 06b9c10dfec6..c699dd21f090 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -465,7 +465,7 @@ Is the change [acceptable for releases][release-acceptable] and do you wish to h - No: Use the `master` branch, do not backport the pull request. - Yes: Can the change be implemented the same way on the `master` and release branches? For example, a packages major version might differ between the `master` and release branches, such that separate security patches are required. - - Yes: Use the `master` branch and [backport the pull request](#backporting-changes). + - Yes: Use the `master` branch and [backport the pull request](#how-to-backport-pull-requests). - No: Create separate pull requests to the `master` and `release-XX.YY` branches. Furthermore, if the change causes a [mass rebuild][mass-rebuild], use the appropriate staging branch instead: diff --git a/nixos/README.md b/nixos/README.md index b3cd9d234fa6..d0257e12d933 100644 --- a/nixos/README.md +++ b/nixos/README.md @@ -21,12 +21,14 @@ Reviewing process: - Ensure that the module maintainers are notified. - [CODEOWNERS](https://help.github.com/articles/about-codeowners/) will make GitHub notify users based on the submitted changes, but it can happen that it misses some of the package maintainers. - Ensure that the module tests, if any, are succeeding. + - You may invoke OfBorg with `@ofborg test ` to build `nixosTests.` - Ensure that the introduced options are correct. - Type should be appropriate (string related types differs in their merging capabilities, `loaOf` and `string` types are deprecated). - Description, default and example should be provided. - Ensure that option changes are backward compatible. - - `mkRenamedOptionModuleWith` provides a way to make option changes backward compatible. -- Ensure that removed options are declared with `mkRemovedOptionModule` + - `mkRenamedOptionModuleWith` provides a way to make renamed option backward compatible. + - Use `lib.versionAtLeast config.system.stateVersion "23.11"` on backward incompatible changes which may corrupt, change or update the state stored on existing setups. +- Ensure that removed options are declared with `mkRemovedOptionModule`. - Ensure that changes that are not backward compatible are mentioned in release notes. - Ensure that documentations affected by the change is updated. @@ -55,6 +57,7 @@ New modules submissions introduce a new module to NixOS. Reviewing process: +- Ensure that all file paths [fit the guidelines](../CONTRIBUTING.md#file-naming-and-organisation). - Ensure that the module tests, if any, are succeeding. - Ensure that the introduced options are correct. - Type should be appropriate (string related types differs in their merging capabilities, `loaOf` and `string` types are deprecated). @@ -76,9 +79,9 @@ Sample template for a new module review is provided below. - [ ] options have default - [ ] options have example - [ ] options have descriptions -- [ ] No unneeded package is added to environment.systemPackages -- [ ] meta.maintainers is set -- [ ] module documentation is declared in meta.doc +- [ ] No unneeded package is added to `environment.systemPackages` +- [ ] `meta.maintainers` is set +- [ ] module documentation is declared in `meta.doc` ##### Possible improvements diff --git a/pkgs/README.md b/pkgs/README.md index 4845cfa31755..ab2598bfcc4a 100644 --- a/pkgs/README.md +++ b/pkgs/README.md @@ -696,16 +696,16 @@ It can happen that non-trivial updates include patches or more complex changes. Reviewing process: -- Ensure that the package versioning fits the guidelines. -- Ensure that the commit text fits the guidelines. +- Ensure that the package versioning [fits the guidelines](#versioning). +- Ensure that the commit text [fits the guidelines](../CONTRIBUTING.md#commit-conventions). - Ensure that the package maintainers are notified. - [CODEOWNERS](https://help.github.com/articles/about-codeowners) will make GitHub notify users based on the submitted changes, but it can happen that it misses some of the package maintainers. -- Ensure that the meta field information is correct. +- Ensure that the meta field information [fits the guidelines](#meta-attributes) and is correct: - License can change with version updates, so it should be checked to match the upstream license. - If the package has no maintainer, a maintainer must be set. This can be the update submitter or a community member that accepts to take maintainership of the package. - Ensure that the code contains no typos. -- Building the package locally. - - pull requests are often targeted to the master or staging branch, and building the pull request locally when it is submitted can trigger many source builds. +- Build the package locally. + - Pull requests are often targeted to the master or staging branch, and building the pull request locally when it is submitted can trigger many source builds. - It is possible to rebase the changes on nixos-unstable or nixpkgs-unstable for easier review by running the following commands from a nixpkgs clone. ```ShellSession @@ -722,7 +722,7 @@ Reviewing process: ```ShellSession $ nix-shell -p nixpkgs-review --run "nixpkgs-review pr PRNUMBER" ``` -- Running every binary. +- Run every binary. Sample template for a package update review is provided below. @@ -731,7 +731,7 @@ Sample template for a package update review is provided below. - [ ] package name fits guidelines - [ ] package version fits guidelines -- [ ] package build on ARCHITECTURE +- [ ] package builds on ARCHITECTURE - [ ] executables tested on ARCHITECTURE - [ ] all depending packages build - [ ] patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed @@ -748,18 +748,20 @@ New packages are a common type of pull requests. These pull requests consists in Review process: -- Ensure that the package versioning fits the guidelines. -- Ensure that the commit name fits the guidelines. -- Ensure that the meta fields contain correct information. +- Ensure that all file paths [fit the guidelines](../CONTRIBUTING.md#file-naming-and-organisation). +- Ensure that the package name and version [fits the guidelines](#package-naming). +- Ensure that the package versioning [fits the guidelines](#versioning). +- Ensure that the commit text [fits the guidelines](../CONTRIBUTING.md#commit-conventions). +- Ensure that the meta fields [fits the guidelines](#meta-attributes) and contain the correct information: - License must match the upstream license. - Platforms should be set (or the package will not get binary substitutes). - Maintainers must be set. This can be the package submitter or a community member that accepts taking up maintainership of the package. - Report detected typos. - Ensure the package source: - - Uses mirror URLs when available. + - Uses `mirror://` URLs when available. - Uses the most appropriate functions (e.g. packages from GitHub should use `fetchFromGitHub`). -- Building the package locally. -- Running every binary. +- Build the package locally. +- Run every binary. Sample template for a new package review is provided below. @@ -769,7 +771,7 @@ Sample template for a new package review is provided below. - [ ] package path fits guidelines - [ ] package name fits guidelines - [ ] package version fits guidelines -- [ ] package build on ARCHITECTURE +- [ ] package builds on ARCHITECTURE - [ ] executables tested on ARCHITECTURE - [ ] `meta.description` is set and fits guidelines - [ ] `meta.license` fits upstream license