Share your saved replies!

GitHub has a mechanism that allows you to save snippets of text in a library to quickly fill out comments and reviews. I’ve found it pretty useful to avoid writing the same boilerplate text over and over again.

If you don’t yet use saved replies, but find yourself writing the same things often when interacting with Nixpkgs, it might be time to go over your more high-quality reviews and copy over the most frequently written snippets into a saved reply or two.

If you already use saved replies and have some snippets that are useful for Nixpkgs contributors, share them here! Some of my own snippets can be found below.

Welcome + Commit Conventions

Many new contributors to Nixpkgs make mistakes when it comes to our commit conventions. I wrote up a customisable paragraph that I use as my review “header” comment for these situations.

Welcome to Nixpkgs! Thanks for packaging this software.

Please review our [Commit Conventions](https://www.github.com/NixOS/nixpkgs/blob/492f874466bf5a11603c78e2ec688de4959bb4ed/CONTRIBUTING.md#commit-conventions) and reword your commits appropriately; their titles must follow very specific patterns as defined in our documentation. Don't forget to squash your intermediate/`fixup` commits.

I've left some additional review comments below.
Non-blocking Nit Disclaimer

Sometimes I’ll make a review comment that is very much optional, and since the discussion in #387072, PR authors now have options when dealing with nits. The snippet below lets them know a review comment is optional, and advises them of the conversation in #387072.

<details><summary><h6> Non-blocking nit: Not required for approval. Apply or ignore at your own discretion.</h6></summary><h6>This proposed nit would be beneficial for the code in question, but it may not be directly related to the changes being made in this pull request. If it's not worth the effort to apply this, simply leave a comment stating this is best left for another pull request.</br></br><i>For more information, consult Issue <a href=https://www.github.com/NixOS/nixpkgs/issues/387072>#387072</a>.</i></h6></details>

Fun fact: you can use the www. prefix before github.com to prevent a linkback from that issue to your review.

This template is based on a review from @Prince213 I saw here.

Various 'this is wrong' reviews

To make derivation reviews easier, I have a collection of snippets that start with Github’s special suggestion code highlighting, where I can highlight what exactly is wrong, how to fix it, and why it’s important.

Wrong hash type
```suggestion
    hash = "sha256-hash";
```
The `sha256` [hash attribute](https://wiki.nixos.org/wiki/Nix_Hash) in fixed-output derivations has been replaced by the new `hash` attribute, which encourages the use of [SRI hashes](https://www.w3.org/TR/SRI/).
Tag instead of revision
```suggestion
    tag = "v${version}";
```

There is a chance that upstream will name a tag with the same name as a `git` HEAD, which can cause issues with this fetcher's fixed output derivation if upstream pushes to that branch. Since we want the tags, which are usually less prone to change, we can use the `tag` attribute that automatically prefixes the value with `refs/tags`, preventing the fetcher from fetching anything that's not a tag.
Wrong meta.maintainers
```suggestion
    maintainers = with lib.maintainers; [ attr ];
```
Maintainers must be declared in [`maintainers/maintainer.list.nix`](https://github.com/NixOS/nixpkgs/blob/492f874466bf5a11603c78e2ec688de4959bb4ed/maintainers/maintainer-list.nix). Please follow the [Maintainers' README](https://github.com/NixOS/nixpkgs/blob/492f874466bf5a11603c78e2ec688de4959bb4ed/maintainers/README.md) to add your entry there.
Wrong pname
```suggestion
  pname = "CORRECTNAMEHERE";
```
Please review the [package naming guidelines](https://github.com/NixOS/nixpkgs/blob/492f874466bf5a11603c78e2ec688de4959bb4ed/pkgs/README.md#package-naming):
- It _should_ be identical to the upstream package name.
- It _must_ not contain uppercase letters.
Wrong version
```suggestion
  version = "CORRECTVERSION";
```
Please review the [package versioning guidelines](https://github.com/NixOS/nixpkgs/blob/492f874466bf5a11603c78e2ec688de4959bb4ed/pkgs/README.md#versioning):
- The version attribute _must_ start with a digit.
- If we're tracking a commit from a repository without a version assigned, then the version attribute should be the latest upstream version preceding that commit, followed by `-unstable-` and the date of the fetched commit. The date must be in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) "YYYY-MM-DD" format.
  - If a project has no suitable preceding releases, then the latest upstream version in the above schema should be `0`.
13 Likes

neat, here are some of mine:

Absolute store path in desktop entries

Please don’t put absolute store paths in Exec and Icon in xdg desktop entries. Desktop environments like GNOME tend to copy such files to ~/.config/autostart when they are marked for automatic startup. Since these copies don’t get their own gcroot their store path references will go inevitably go stale. Please use the basename of executables (assuming its available in $out/bin/ and $PATH) and icons ($out/share/{pixmap,icons}) instead.

Explain IFD and why we disallow it

Import From Derivation (IFD) is disallowed in Nixpkgs for performance reasons.

A nix build is split into two phases, evaluation and build. Evaluation is when the nix code is parsed and turned into derivation files, and the build operate on those derivation files to realize (build) or substitute (fetch from cache) them into nix store paths. The problem with IFD is that the evaluation runs into a stop when trying to import more nix code from the store, and has to build that store path before being able to continue the evaluation. Not only is this conceptually a blocking operation, but the nix runtime is very poorly optimized when it comes to switching between evaluations and builds.

Hydra regularly evaluate the entire nixpkgs package set, and sequential builds during evaluation would increase evaluation times to the points where it become impractical.

maintainer-list.nix commit in separate commit

Please make sure you add yourself to maintainer-list.nix in a separate commit, titled maintainers: add <your handle> in line the the commit conventions. Make sure that this commit precedes any other commit where you make use of that new maintainer entry. To achieve this you need to will likely have to rewrite the git history and perform a --force(-with-lease) push.

Welcome new maintainer

Thank you and welcome to nixpkgs!

Now that you’ve added yourself to maintainer-list.nix you should in short time be invited to join the nixos/nixpkgs-maintainers group. That invite email will expire in a week. Joining the group will give you additional permissions in nixpkgs, like the ability to change labels on issues and pull requests.

6 Likes

What a brilliant idea! Thanks for doing this.

2 Likes