summary refs log tree commit diff
path: root/nixos/doc/manual/development
diff options
context:
space:
mode:
authorEric Sagnes <eric.sagnes@gmail.com>2016-11-22 23:15:02 +0900
committerFranz Pletz <fpletz@fnordicwalking.de>2016-11-22 15:15:02 +0100
commit2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34 (patch)
tree49fecd8addbd8cd8a102cd73ca040ad0d1f74b9f /nixos/doc/manual/development
parentd94e93ccdf1671f6c50f48ac37d4b5d1210cb481 (diff)
downloadnixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar.gz
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar.bz2
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar.lz
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar.xz
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.tar.zst
nixlib-2b1d67a275a49bac3264cdfc0c0a5e2c540c9a34.zip
manual: reviewing contributions nixos -> nixpkgs (#20626)
Diffstat (limited to 'nixos/doc/manual/development')
-rw-r--r--nixos/doc/manual/development/development.xml1
-rw-r--r--nixos/doc/manual/development/reviewing-contributions.xml393
2 files changed, 0 insertions, 394 deletions
diff --git a/nixos/doc/manual/development/development.xml b/nixos/doc/manual/development/development.xml
index b0364b346577..47343d93cde9 100644
--- a/nixos/doc/manual/development/development.xml
+++ b/nixos/doc/manual/development/development.xml
@@ -18,7 +18,6 @@ NixOS.</para>
 <xi:include href="building-nixos.xml" />
 <xi:include href="nixos-tests.xml" />
 <xi:include href="testing-installer.xml" />
-<xi:include href="reviewing-contributions.xml" />
 <xi:include href="releases.xml" />
 
 </part>
diff --git a/nixos/doc/manual/development/reviewing-contributions.xml b/nixos/doc/manual/development/reviewing-contributions.xml
deleted file mode 100644
index f86928bcd5d0..000000000000
--- a/nixos/doc/manual/development/reviewing-contributions.xml
+++ /dev/null
@@ -1,393 +0,0 @@
-<chapter xmlns="http://docbook.org/ns/docbook"
-        xmlns:xlink="http://www.w3.org/1999/xlink"
-        xmlns:xi="http://www.w3.org/2001/XInclude"
-        version="5.0"
-        xml:id="sec-reviewing-contributions">
-
-<title>Reviewing contributions</title>
-
-<warning>
-  <para>The following section is a draft and reviewing policy is still being 
-    discussed.</para>
-</warning>
-
-<para>The nixpkgs projects receives a fairly high number of contributions via 
-  GitHub pull-requests. Reviewing and approving these is an important task and a 
-  way to contribute to the project.</para>
-
-<para>The high change rate of nixpkgs make any pull request that is open for 
-  long enough subject to conflicts that will require extra work from the 
-  submitter or the merger. Reviewing pull requests in a timely manner and being 
-  responsive to the comments is the key to avoid these. Github provides sort 
-  filters that can be used to see the <link 
-    xlink:href="https://github.com/NixOS/nixpkgs/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc">most 
-    recently</link> and the <link 
-    xlink:href="https://github.com/NixOS/nixpkgs/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-asc">least 
-    recently</link> updated pull-requests.</para>
-
-<para>When reviewing a pull request, please always be nice and polite. 
-  Controversial changes can lead to controversial opinions, but it is important 
-  to respect every community members and their work.</para>
-
-<para>GitHub provides reactions, they are a simple and quick way to provide 
-  feedback to pull-requests or any comments. The thumb-down reaction should be 
-  used with care and if possible accompanied with some explanations so the 
-  submitter has directions to improve his contribution.</para>
-
-<para>Pull-requests reviews should include a list of what has been reviewed in a 
-  comment, so other reviewers and mergers can know the state of the 
-  review.</para>
-
-<para>All the review template samples provided in this section are generic and 
-  meant as examples. Their usage is optional and the reviewer is free to adapt 
-  them to his liking.</para>
-
-<section><title>Package updates</title>
-
-<para>A package update is the most trivial and common type of pull-request. 
-  These pull-requests mainly consist in updating the version part of the package 
-  name and the source hash.</para>
-<para>It can happen that non trivial updates include patches or more complex 
-  changes.</para>
-
-<para>Reviewing process:</para>
-
-<itemizedlist>
-  <listitem><para>Add labels to the pull-request. (Requires commit 
-      rights)</para>
-    <itemizedlist>
-      <listitem><para><literal>8.has: package (update)</literal> and any topic 
-          label that fit the updated package.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the package versioning is fitting the 
-      guidelines.</para></listitem>
-  <listitem><para>Ensure that the commit text is fitting the 
-      guidelines.</para></listitem>
-  <listitem><para>Ensure that the package maintainers are notified.</para>
-    <itemizedlist>
-      <listitem><para>mention-bot usually notify GitHub users based on the 
-          submitted changes, but it can happen that it misses some of the 
-          package maintainers.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the meta field contains correct 
-      information.</para>
-    <itemizedlist>
-      <listitem><para>License can change with version updates, so it should be 
-          checked to be fitting upstream license.</para></listitem>
-      <listitem><para>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.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the code contains no typos.</para></listitem>
-  <listitem><para>Building the package locally.</para>
-    <itemizedlist>
-      <listitem><para>Pull-requests are often targeted to the master or staging 
-          branch so building the pull-request locally as it is submitted can 
-          trigger a large amount of source builds.</para>
-        <para>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.
-<screen>
-$ git remote add channels https://github.com/NixOS/nixpkgs-channels.git <co 
-  xml:id='reviewing-rebase-1' />
-$ git fetch channels nixos-unstable <co xml:id='reviewing-rebase-2' />
-$ git fetch origin pull/PRNUMBER/head <co xml:id='reviewing-rebase-3' />
-$ git rebase --onto nixos-unstable BASEBRANCH FETCH_HEAD <co 
-  xml:id='reviewing-rebase-4' />
-</screen>
-        <calloutlist>
-          <callout arearefs='reviewing-rebase-1'>
-            <para>This should be done only once to be able to fetch channel 
-              branches from the nixpkgs-channels repository.</para>
-          </callout>
-          <callout arearefs='reviewing-rebase-2'>
-            <para>Fetching the nixos-unstable branch.</para>
-          </callout>
-          <callout arearefs='reviewing-rebase-3'>
-            <para>Fetching the pull-request changes, <varname>PRNUMBER</varname> 
-              is the number at the end of the pull-request title and 
-              <varname>BASEBRANCH</varname> the base branch of the 
-              pull-request.</para>
-          </callout>
-          <callout arearefs='reviewing-rebase-3'>
-            <para>Rebasing the pull-request changes to the nixos-unstable 
-              branch.</para>
-          </callout>
-        </calloutlist>
-        </para>
-      </listitem>
-      <listitem>
-        <para>The <link xlink:href="https://github.com/madjar/nox">nox</link> 
-          tool can be used to review a pull-request content in a single command. 
-          It doesn't rebase on a channel branch so it might trigger multiple 
-          source builds. <varname>PRNUMBER</varname> should be replaced by the 
-          number at the end of the pull-request title.</para>
-<screen>
-$ nix-shell -p nox --run "nox-review -k pr PRNUMBER"
-</screen>
-      </listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Running every binary.</para></listitem>
-</itemizedlist>
-
-<example><title>Sample template for a package update review</title>
-<screen>
-##### Reviewed points
-
-- [ ] package name fits guidelines
-- [ ] package version fits guidelines
-- [ ] package build on ARCHITECTURE
-- [ ] executables tested on ARCHITECTURE
-- [ ] all depending packages build
-
-##### Possible improvements
-
-##### Comments
-
-</screen></example>
-</section>
-
-<section><title>New packages</title>
-
-<para>New packages are a common type of pull-requests. These pull requests 
-  consists in adding a new nix-expression for a package.</para>
-
-<para>Reviewing process:</para>
-
-<itemizedlist>
-  <listitem><para>Add labels to the pull-request. (Requires commit 
-      rights)</para>
-    <itemizedlist>
-      <listitem><para><literal>8.has: package (new)</literal> and any topic 
-          label that fit the new package.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the package versioning is fitting the 
-      guidelines.</para></listitem>
-  <listitem><para>Ensure that the commit name is fitting the 
-      guidelines.</para></listitem>
-  <listitem><para>Ensure that the meta field contains correct 
-      information.</para>
-    <itemizedlist>
-      <listitem><para>License must be checked to be fitting upstream 
-          license.</para></listitem>
-      <listitem><para>Platforms should be set or the package will not get binary 
-          substitutes.</para></listitem>
-      <listitem><para>A maintainer must be set, this can be the package 
-          submitter or a community member that accepts to take maintainership of 
-          the package.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the code contains no typos.</para></listitem>
-  <listitem><para>Ensure the package source.</para>
-    <itemizedlist>
-      <listitem><para>Mirrors urls should be used when 
-          available.</para></listitem>
-      <listitem><para>The most appropriate function should be used (e.g. 
-          packages from GitHub should use 
-          <literal>fetchFromGitHub</literal>).</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Building the package locally.</para></listitem>
-  <listitem><para>Running every binary.</para></listitem>
-</itemizedlist>
-
-<example><title>Sample template for a new package review</title>
-<screen>
-##### Reviewed points
-
-- [ ] package path fits guidelines
-- [ ] package name fits guidelines
-- [ ] package version fits guidelines
-- [ ] package build on ARCHITECTURE
-- [ ] executables tested on ARCHITECTURE
-- [ ] `meta.description` is set and fits guidelines
-- [ ] `meta.license` fits upstream license
-- [ ] `meta.platforms` is set
-- [ ] `meta.maintainers` is set
-- [ ] build time only dependencies are declared in `nativeBuildInputs`
-- [ ] source is fetched using the appropriate function
-- [ ] phases are respected
-- [ ] patches that are remotely available are fetched with `fetchpatch`
-
-##### Possible improvements
-
-##### Comments
-
-</screen></example>
-</section>
-
-<section><title>Module updates</title>
-
-<para>Module updates are submissions changing modules in some ways. These often 
-  contains changes to the options or introduce new options.</para>
-
-<para>Reviewing process</para>
-
-<itemizedlist>
-  <listitem><para>Add labels to the pull-request. (Requires commit 
-      rights)</para>
-    <itemizedlist>
-      <listitem><para><literal>8.has: module (update)</literal> and any topic 
-          label that fit the module.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the module maintainers are notified.</para>
-    <itemizedlist>
-      <listitem><para>Mention-bot notify GitHub users based on the submitted 
-          changes, but it can happen that it miss some of the package 
-          maintainers.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the module tests, if any, are 
-      succeeding.</para></listitem>
-  <listitem><para>Ensure that the introduced options are correct.</para>
-    <itemizedlist>
-      <listitem><para>Type should be appropriate (string related types differs 
-          in their merging capabilities, <literal>optionSet</literal> and 
-          <literal>string</literal> types are deprecated).</para></listitem>
-      <listitem><para>Description, default and example should be 
-          provided.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that option changes are backward compatible.</para>
-    <itemizedlist>
-      <listitem><para><literal>mkRenamedOptionModule</literal> and 
-          <literal>mkAliasOptionModule</literal> functions provide way to make 
-          option changes backward compatible.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that removed options are declared with 
-      <literal>mkRemovedOptionModule</literal></para></listitem>
-  <listitem><para>Ensure that changes that are not backward compatible are 
-      mentioned in release notes.</para></listitem>
-  <listitem><para>Ensure that documentations affected by the change is 
-      updated.</para></listitem>
-</itemizedlist>
-
-<example><title>Sample template for a module update review</title>
-<screen>
-##### Reviewed points
-
-- [ ] changes are backward compatible
-- [ ] removed options are declared with `mkRemovedOptionModule`
-- [ ] changes that are not backward compatible are documented in release notes
-- [ ] module tests succeed on ARCHITECTURE
-- [ ] options types are appropriate
-- [ ] options description is set
-- [ ] options example is provided
-- [ ] documentation affected by the changes is updated
-
-##### Possible improvements
-
-##### Comments
-
-</screen></example>
-</section>
-
-<section><title>New modules</title>
-
-<para>New modules submissions introduce a new module to NixOS.</para>
-
-<itemizedlist>
-  <listitem><para>Add labels to the pull-request. (Requires commit 
-      rights)</para>
-    <itemizedlist>
-      <listitem><para><literal>8.has: module (new)</literal> and any topic label 
-          that fit the module.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the module tests, if any, are 
-      succeeding.</para></listitem>
-  <listitem><para>Ensure that the introduced options are correct.</para>
-    <itemizedlist>
-      <listitem><para>Type should be appropriate (string related types differs 
-          in their merging capabilities, <literal>optionSet</literal> and 
-          <literal>string</literal> types are deprecated).</para></listitem>
-      <listitem><para>Description, default and example should be 
-          provided.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that module <literal>meta</literal> field is 
-      present</para>
-    <itemizedlist>
-      <listitem><para>Maintainers should be declared in 
-          <literal>meta.maintainers</literal>.</para></listitem>
-      <listitem><para>Module documentation should be declared with 
-          <literal>meta.doc</literal>.</para></listitem>
-    </itemizedlist>
-  </listitem>
-  <listitem><para>Ensure that the module respect other modules 
-      functionality.</para>
-    <itemizedlist>
-      <listitem><para>For example, enabling a module should not open firewall 
-          ports by default.</para></listitem>
-    </itemizedlist>
-  </listitem>
-</itemizedlist>
-
-<example><title>Sample template for a new module review</title>
-<screen>
-##### Reviewed points
-
-- [ ] module path fits the guidelines
-- [ ] module tests succeed on ARCHITECTURE
-- [ ] options have appropriate types
-- [ ] options have default
-- [ ] options have example
-- [ ] options have descriptions
-- [ ] No unneeded package is added to system.environmentPackages
-- [ ] meta.maintainers is set
-- [ ] module documentation is declared in meta.doc
-
-##### Possible improvements
-
-##### Comments
-
-</screen></example>
-</section>
-
-<section><title>Other submissions</title>
-
-<para>Other type of submissions requires different reviewing steps.</para>
-
-<para>If you consider having enough knowledge and experience in a topic and 
-  would like to be a long-term reviewer for related submissions, please contact 
-  the current reviewers for that topic. They will give you information about the 
-  reviewing process.
-The main reviewers for a topic can be hard to find as there is no list, but 
-checking past pull-requests to see who reviewed or git-blaming the code to see 
-who committed to that topic can give some hints.</para>
-
-<para>Container system, boot system and library changes are some examples of the 
-  pull requests fitting this category.</para>
-
-</section>
-
-<section><title>Merging pull-requests</title>
-
-<para>It is possible for community members that have enough knowledge and 
-  experience on a special topic to contribute by merging pull requests.</para>
-
-<para>TODO: add the procedure to request merging rights.</para>
-
-<!--
-The following paragraph about how to deal with unactive contributors is just a
-proposition and should be modified to what the community agrees to be the right
-policy.
-
-<para>Please note that contributors with commit rights unactive for more than 
-  three months will have their commit rights revoked.</para>
--->
-
-<para>In a case a contributor leaves definitively the Nix community, he should 
-  create an issue or notify the mailing list with references of packages and 
-  modules he maintains so the maintainership can be taken over by other 
-  contributors.</para>
-
-</section>
-</chapter>