Clean up contributing documentation #245243

infinisil · 2023-07-24T21:19:51Z

Motivation

For RFC 140 (implementation part 1) I want to update documentation on how to contribute packages, see here.

However, after taking a closer look, contributing documentation is currently missing, unorganized, scattered and/or duplicated throughout various documents:

A Contributing section in the top-level README.md containing some minimal info about branches
The CONTRIBUTING.md file, which GitHub prominently links to when you open a pull request, containing some duplicated but also updated copies from the contributing section in the manual.
The Contributing section in the manual (rendered), containing a bunch of general docs, but also a lot of docs specific to pkgs. In fact that section sometimes assumes everybody just wants to make changes to pkgs
Various sections in the Nixpkgs manual for contributing to specific parts, like this one for Python (which has taken the #contributing anchor..), or this one for R
Various documents throughout the source code not included in the manual, like this one for Haskell, or here for nextcloud.

Instead of trying to shoehorn the documentation for RFC 140 into this mess, it would be better to first clean up the overall structure. Part of this was already discussed in #244056, and this PR will attempt to implement this roughly.

Goal

The following organization is proposed:

README.md: A non-technical contribution introduction, links to CONTRIBUTING.md
CONTRIBUTING.md: General contribution guidelines that apply to all of Nixpkgs, such as:
- The file structure of Nixpkgs, linking to more detailed README.md's for individual parts
- Different ways of contributing: Proposing changes, Reviewing changes, Merging changes
- Flow of changes from PR merge to release (including staging)
- Coding Conventions (syntax, formatting, filenames)
**/README.md: Contributing documentation specific to other parts of Nixpkgs, also linking to CONTRIBUTING.md. This should include how to modify, test and document each part of the code base.
- pkgs/README.md: Contribution documentation for packages (creating, updating, testing packages, etc.)
- doc/README.md: Contribution documentation for the user manual
- lib/README.md: Contributing to lib, already done with Create a Readme in lib #244044
- ...
Nixpkgs manual: No contributing docs anymore, only link to CONTRIBUTING.md and only include user-level documentation:
- How to report bugs or request a change (using the issue tracker)
- How to test pull requests (without reviewing the code)
- How to check if merged pull requests are available in releases

This PR will attempt to set the baseline for this by at least moving the existing documentation to the right place and making it work together.

Best reviewed commit-by-commit!

Todo

Define structure of CONTRIBUTING.md
Roughly move all docs relevant to all of Nixpkgs to CONTRIBUTING.md
Move all docs relevant to pkgs into a new pkgs/README.md
Clean up and reconcile moved documentation
- I couldn't clean up everything, but I made a good effort for at least CONTRIBUTING.md
Ensure everything is cross-linked nicely
Ensure that removed anchors in the manual keep working
Add myself as a code owner to all relevant files
- See also Add myself as a code owner of the contributing documentation #248166

CONTRIBUTING.md

nixos-discourse · 2023-07-27T14:36:28Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-27-documentation-team-meeting-notes-67/30998/1

nixos-discourse · 2023-08-08T15:29:32Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-08-nixpkgs-architecture-team-meeting-42/31454/1

matthiasbeyer · 2023-08-08T20:16:26Z

Feel free to cherry-pick #248013 onto this PR and close it. 👍

infinisil · 2023-08-09T17:56:14Z

@matthiasbeyer #248013 (comment)

I want to take ownership of this part of the documentation, especially with the cleanup in NixOS#245243. Doing so before that PR is merged also allows me to get notified about any potential future merge conflicts before they happen.

Not used anymore after b3fa79b

maintainers/README.md

FRidh · 2023-08-14T06:44:43Z

Nice work! Good to see this happening.

Various sections in the Nixpkgs manual for contributing to specific parts, like this one for Python (which has taken the #contributing anchor..), or this one for R

As an example, where would the Python-specific contributing part go to? **/README.md? And how is that to be discovered then by say somebody new that wants to contribute? I suggest having a section in CONTRIBUTING.md, like the Nixpkgs manual, that covers the larger/common languages/frameworks, essentially just linking to the corresponding README.md.

Maybe after that we could use a tool (e.g. sphinx or whatever we use now for the manual) to compile the various README.md into a contributing manual.

zimbatm

This is moving in the right direction and is clearly better than what we had before.

Users are now able to navigate the repository and find out about the related documentation, instead of going to a hidden nixpkgs manual URL.

maintainers/README.md

CONTRIBUTING.md

FRidh · 2023-08-14T10:30:15Z

doc/contributing/coding-conventions.chapter.md

-    assert system == "i686-linux";
-      stdenv.mkDerivation { ...
-  ```
+This section has been moved to [CONTRIBUTING.md](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md).


These types of links, while useful when browsing through a browser, now hardcode the branch. I think you can just use relative links.

We can't use relative links because this is the rendered manual to be shown on nixos.org where the source isn't available.

minijackson

Great stuff, thanks a lot!

minijackson · 2023-08-14T11:26:54Z

CONTRIBUTING.md

+   1. [Fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo#forking-a-repository) the [Nixpkgs repository](https://github.com/nixos/nixpkgs/).
+   1. [Clone the forked repository](https://docs.github.com/en/get-started/quickstart/fork-a-repo#cloning-your-forked-repository) into a local `nixpkgs` directory.
+   1. [Configure the upstream Nixpkgs repository](https://docs.github.com/en/get-started/quickstart/fork-a-repo#configuring-git-to-sync-your-fork-with-the-upstream-repository).


works when rendered in through GitHub, but I think you meant 1., 2., 3.

Yeah that's intentional, I don't really mind either way, but that's an explicitly supported GitHub Markdown feature (and I think effectively all markdown flavors support this)

Ok good to know. I was just confused when reading through the source code.

CONTRIBUTING.md

minijackson · 2023-08-14T11:46:36Z

CONTRIBUTING.md

+    checkout staging-next
+    commit id:"fixup "
+    checkout master
+    merge staging-next type:HIGHLIGHT id:"manual (PR)"


Really nice diagram, much clearer. Thanks!

minijackson · 2023-08-14T11:57:48Z

CONTRIBUTING.md

+| branch | `master` | `staging` | `staging-next` |
+| --- | --- | --- | --- |
+| Used for development | :heavy_check_mark: | :heavy_check_mark: | :x: |
+| Built by Hydra | :heavy_check_mark: | :x: | :heavy_check_mark: |
+| [Mass rebuilds](#mass-rebuilds) | :x: | :heavy_check_mark: | :warning: Only to fix Hydra builds |
+| Critical security fixes | :heavy_check_mark: for non-mass-rebuilds | :x: | :heavy_check_mark: for mass-rebuilds |
+| Automatically merged into | `staging-next` | - | `staging` |
+| Manually merged into | - | `staging-next` | `master` |


Somehow, some heavy_check_marks gets replaced with the green one (image), and some other with the black one (emoji), which is harder to run with the dark theme. Not sure what's happening

Yeah I saw that too, super weird. Switching to emoji's seems to make it work correctly though, so I did that: eb75810 😄

CONTRIBUTING.md

minijackson · 2023-08-14T12:52:10Z

CONTRIBUTING.md

+
+#### Meets Nixpkgs contribution standards
+
+The last checkbox is fits the guidelines in this `CONTRIBUTING.md` file. The contributing document has detailed information on standards the Nix community has for commit messages, reviews, licensing of contributions you make to the project, etc... Everyone should read and understand the standards the community has for contributing before submitting a pull request.


This is a really nice writup for the PR template checkboxes. I think it would be nice to change the PR template itself to indicate that there's this documentation here.

Yeah, I am intending to update the pull request template with future PRs, let's not do it in this PR though

CONTRIBUTING.md

minijackson · 2023-08-14T13:02:18Z

CONTRIBUTING.md


-It is possible for community members that have enough knowledge and experience on a special topic to contribute by merging pull requests.
+- When changing the bootloader installation process, extra care must be taken. Grub installations cannot be rolled back, hence changes may break people’s installations forever. For any non-trivial change to the bootloader please file a PR asking for review, especially from \@edolstra.


I think it's a bit weird to have this point in the "Commit conventions" section.

Oh yeah, I initially wasn't sure where it would fit, but now I'm realizing that nixos/README.md would be great: f22e4a3 (of course, that document also desperately needs more content and a cleanup, but that's future work)

minijackson · 2023-08-14T13:10:56Z

pkgs/README.md

+- Run package-internal tests with `nix-build --attr pkgs.PACKAGE.passthru.tests`
+- Run [NixOS tests](https://nixos.org/manual/nixos/unstable/#sec-nixos-tests) with `nix-build --attr nixosTest.NAME`, where `NAME` is the name of the test listed in `nixos/tests/all-tests.nix`
+- Run [global package tests](https://nixos.org/manual/nixpkgs/unstable/#sec-package-tests) with `nix-build --attr tests.PACKAGE`, where `PACKAGE` is the name of the test listed in `pkgs/test/default.nix`
+- See `lib/tests/NAME.nix` for instructions on running specific library tests


It may be a good place to mention nixpkgs-review again, for completeness' sake

Good point, I'd like to defer that to the future when I get to cleaning up the pull request template and the its corresponding section

nixos-discourse · 2023-08-14T14:53:54Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-14-documentation-team-meeting-notes-72/31722/1

infinisil · 2023-08-14T16:08:14Z

I suggest having a section in CONTRIBUTING.md, like the Nixpkgs manual, that covers the larger/common languages/frameworks, essentially just linking to the corresponding README.md.

@FRidh This would fit into the new pkgs/README.md, but that can be in a future change.

Maybe after that we could use a tool (e.g. sphinx or whatever we use now for the manual) to compile the various README.md into a contributing manual.

I like the idea! It's a bit ironic to move away from rendering the docs ourselves only to render them ourselves again later, but it does make sense when looking at the whole thing :P

- Contributing without a GitHub account - Mention OfBorg - nix.useSandbox -> nix.settings.sandbox - nixpkgs-review is good for not just version updates Co-authored-by: Rémi NICOLE <minijackson@users.noreply.github.com>

- Fix sentence about meeting contributing standards - pkgs -> packages - Use emoji's because GitHub renders the :*: things weird sometimes - Move a dot Co-authored-by: Rémi NICOLE <minijackson@users.noreply.github.com>

This needs a rewrite at some point..

infinisil · 2023-08-14T17:57:41Z

I did a minor Git history cleanup by squashing all the single typo commits and other minor changes together. This is ready to be merged imo (with a merge commit ideally).

nixos-discourse · 2023-08-17T20:29:40Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-17-documentation-team-meeting-notes-73/31853/1

nixos-discourse · 2023-08-23T07:38:16Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-21-documentation-team-meeting-notes-74/32083/1

nixos-discourse · 2023-09-03T16:40:10Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/this-month-in-nix-docs-4-july-august-2023/32502/4

github-actions bot added the 8.has: documentation label Jul 24, 2023

This was referenced Jul 24, 2023

[RFC 140] Simple package paths, part 1b: Enabling the directory structure #237439

Merged

Nixpkgs manual: Don't render Contributing section #244056

Closed

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jul 24, 2023

RaitoBezarius reviewed Jul 24, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

RaitoBezarius reviewed Jul 24, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

infinisil force-pushed the contributing-combining branch from 36278ed to ac80f7d Compare July 25, 2023 16:36

github-actions bot added the 6.topic: nixos label Jul 25, 2023

ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 labels Jul 25, 2023

infinisil mentioned this pull request Aug 2, 2023

CONTRIBUTING.md: add a section about including patches #246766

Open

12 tasks

infinisil force-pushed the contributing-combining branch 5 times, most recently from 6f37135 to cedc57b Compare August 2, 2023 22:30

infinisil assigned roberth Aug 3, 2023

infinisil force-pushed the contributing-combining branch 3 times, most recently from cd21c47 to 8d4d808 Compare August 7, 2023 19:10

Janik-Haag mentioned this pull request Aug 8, 2023

CONTRIBUTING: Add note about how to commit new maintainers-list.nix entry #248013

Merged

infinisil mentioned this pull request Aug 9, 2023

Add myself as a code owner of the contributing documentation #248166

Merged

roberth mentioned this pull request Aug 12, 2023

nixos/install-tools: Add manpages to packages instead of seperating them #248701

Merged

13 tasks

doc/contributing: Remove unreferenced legacy diagram

be26d79

Not used anymore after b3fa79b

github-actions bot added the 6.topic: policy discussion label Aug 14, 2023

matthiasbeyer reviewed Aug 14, 2023

View reviewed changes

maintainers/README.md Show resolved Hide resolved

zimbatm approved these changes Aug 14, 2023

View reviewed changes

maintainers/README.md Show resolved Hide resolved

FRidh reviewed Aug 14, 2023

View reviewed changes

minijackson reviewed Aug 14, 2023

View reviewed changes

FRidh approved these changes Aug 14, 2023

View reviewed changes

minijackson approved these changes Aug 14, 2023

View reviewed changes

infinisil and others added 3 commits August 14, 2023 19:49

CONTRIBUTING.md: Minor content updates based on reviews

237799a

- Contributing without a GitHub account - Mention OfBorg - nix.useSandbox -> nix.settings.sandbox - nixpkgs-review is good for not just version updates Co-authored-by: Rémi NICOLE <minijackson@users.noreply.github.com>

CONTRIBUTING.md: Typos and formatting

2ce1e0b

- Fix sentence about meeting contributing standards - pkgs -> packages - Use emoji's because GitHub renders the :*: things weird sometimes - Move a dot Co-authored-by: Rémi NICOLE <minijackson@users.noreply.github.com>

CONTRIBUTING.md: Move boot loader-specific sentence to pkgs/README.md

de5a39f

This needs a rewrite at some point..

infinisil force-pushed the contributing-combining branch from f22e4a3 to de5a39f Compare August 14, 2023 17:56

infinisil merged commit 50d1165 into NixOS:master Aug 14, 2023
5 of 6 checks passed

infinisil deleted the contributing-combining branch August 14, 2023 19:06

infinisil mentioned this pull request Aug 20, 2023

Documentation: Restructure staging workflow description #243177

Closed

3 tasks

emilylange mentioned this pull request Aug 24, 2023

bark: init at unstable-2023-08-22 #251208

Merged

12 tasks

fricklerhandwerk mentioned this pull request Sep 1, 2023

CONTRIBUTING.md and nixpkgs manual contributing section are redundant #184971

Closed

infinisil mentioned this pull request Sep 5, 2023

[23.05] Backport simple package paths: part 1b #253442

Merged

2 tasks

infinisil mentioned this pull request Nov 26, 2023

rl-23.11: New outline proposal #270257

Closed

fricklerhandwerk mentioned this pull request Dec 10, 2023

Move contributor-facing documentation into the code NixOS/nix#9583

Open

2 tasks

infinisil mentioned this pull request Dec 13, 2023

[CONTINUING] Add "contributing to Nixpkgs" tutorial NixOS/nix.dev#831

Closed

infinisil added this to the RFC 140 milestone Jan 10, 2024

kachick mentioned this pull request Apr 3, 2024

PULL_REQUEST_TEMPLATE.md: Update link to package tests #301318

Merged

13 tasks

abathur mentioned this pull request May 31, 2024

doc: add stdenv passthru chapter #315909

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up contributing documentation #245243

Clean up contributing documentation #245243

infinisil commented Jul 24, 2023 •

edited

Loading

nixos-discourse commented Jul 27, 2023

nixos-discourse commented Aug 8, 2023

matthiasbeyer commented Aug 8, 2023

infinisil commented Aug 9, 2023

FRidh commented Aug 14, 2023 •

edited

Loading

zimbatm left a comment

FRidh Aug 14, 2023

infinisil Aug 14, 2023

minijackson left a comment

minijackson Aug 14, 2023

infinisil Aug 14, 2023

minijackson Aug 14, 2023

minijackson Aug 14, 2023

minijackson Aug 14, 2023

infinisil Aug 14, 2023

minijackson Aug 14, 2023

infinisil Aug 14, 2023

minijackson Aug 14, 2023

infinisil Aug 14, 2023

minijackson Aug 14, 2023

infinisil Aug 14, 2023

nixos-discourse commented Aug 14, 2023

infinisil commented Aug 14, 2023 •

edited

Loading

infinisil commented Aug 14, 2023 •

edited

Loading

nixos-discourse commented Aug 17, 2023

nixos-discourse commented Aug 23, 2023

nixos-discourse commented Sep 3, 2023


		#### Meets Nixpkgs contribution standards

		The last checkbox is fits the guidelines in this `CONTRIBUTING.md` file. The contributing document has detailed information on standards the Nix community has for commit messages, reviews, licensing of contributions you make to the project, etc... Everyone should read and understand the standards the community has for contributing before submitting a pull request.


		It is possible for community members that have enough knowledge and experience on a special topic to contribute by merging pull requests.
		- When changing the bootloader installation process, extra care must be taken. Grub installations cannot be rolled back, hence changes may break people’s installations forever. For any non-trivial change to the bootloader please file a PR asking for review, especially from \@edolstra.

Clean up contributing documentation #245243

Clean up contributing documentation #245243

Conversation

infinisil commented Jul 24, 2023 • edited Loading

Motivation

Goal

Todo

nixos-discourse commented Jul 27, 2023

nixos-discourse commented Aug 8, 2023

matthiasbeyer commented Aug 8, 2023

infinisil commented Aug 9, 2023

FRidh commented Aug 14, 2023 • edited Loading

zimbatm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minijackson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nixos-discourse commented Aug 14, 2023

infinisil commented Aug 14, 2023 • edited Loading

infinisil commented Aug 14, 2023 • edited Loading

nixos-discourse commented Aug 17, 2023

nixos-discourse commented Aug 23, 2023

nixos-discourse commented Sep 3, 2023

infinisil commented Jul 24, 2023 •

edited

Loading

FRidh commented Aug 14, 2023 •

edited

Loading

infinisil commented Aug 14, 2023 •

edited

Loading

infinisil commented Aug 14, 2023 •

edited

Loading