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

https://discourse.nixos.org/t/prs-ready-for-review/3032/216

/marvin opt-in
/status needs_reviewer

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

Bot mistake.
/status needs_reviewer

Also I don't use any of those desktops and I don't know anyone who does, so I'm bouncing this to someone else for testing

/status needs_reviewer

tested on xfce.

/status needs_merger

Also I don't use any of those desktops and I don't know anyone who does, so I'm bouncing this to someone else for testing

This might be a bit controversial, but I would say runtime testing is at most a "nice to have" for a review. In my mind, the reviewers job is to check that the actual diff is good and does not obviously break anything (i.e. breaks at build or test time). For the runtime testing its sufficient to trust the PR author.

Also if you do want find someone who could test this, it would be preferable to actively look someone up (for example with git log) and ping them. Setting it back to needs_reviewer will only assign some other random reviewer which likely doesn't have the needed expertise / setup either.

Anyway, thanks for the reviews @fgaz and @symphorien :)

For the runtime testing its sufficient to trust the PR author.

In most cases including this one maybe yes, but what about new authors and init prs?

For the runtime testing its sufficient to trust the PR author.

In most cases including this one maybe yes, but what about new authors and init prs?

Init PRs are very low-stake: The worst that's going to happen is that we have a new not-quite-working package in the repository. Basically the author has no incentive to do that on purpose, so its relatively unlikely. They will have tested it at least for their use-case. If it does happen, someone is going to notice at some point and fix it / disable it. Either way the worst-case is both "not so bad" and "pretty unlikely".

New authors (or anyone) making changes to existing things might be a bit more tricky, since here the worst case is that it breaks someones setup and the testing burden is "any usecase some nixpkgs user might currently have for the package" which the PR author probably didn't test for. Still, I'd argue that static checks (i.e. builds fine and tests pass) are sufficient in most cases. If something does break on runtime, that's why the channel is called unstable. Its a motivation for the package maintainer to add more/better static checks.

This basically goes back to my personal definition of the unstable channel: We have tested that nothing obvious breaks (nix-review), but things might break on runtime. That's similar to what most other distros call "unstable", with the added benefit of easy rollbacks.