PRs in distress

Development
62

In the PR linked below we are trying to get RFC42 style settings into the syncthing NixOS module. We have some disagreements there unfortunately, so we’d appreciate if someone a bit experienced with NixOS modules would share their opinion and help us make a decision.

This PR is also blocking a dependent PR of mine, that fixes a very annoying bug in the module, and that was very hard to debug. I have been merging those 2 PRs to my nixpkgs fork for a few months now.

2 Likes
63
64
65
66

I have a question on this topic, together with two example cases:

What’s the protocol on PRs for software that requires specific hardware and/or setup? I have two PRs open (one is recent and the other one multiple months old) that functionality-wise are hard to review due to their hardware requirements.

Recent example is [edit: has now been merged] a new package/module that was requested: goxlr-utility: init at 0.12.6 (fixes #243701) by errnoh · Pull Request #249860 · NixOS/nixpkgs · GitHub for GoXLR product line audio mixers. The person who requested the feature is active enough that I can likely get them to test the functionality, but I can imagine cases where even that is not available, what’s the protocol in situations where one wants to add a new package that can’t be tested without specific hardware?

The one that’s been hanging for months is luksroot: fix issue when yubikey is detached during boot process by errnoh · Pull Request #228144 · NixOS/nixpkgs · GitHub where there’s a repeatable corner case functionality for niche setup that involves rotating Yubikey full disk encryption. The problem and solution has been thoroughly explained, fix is simple, but as it involves hardware security it’s not like it should just be rubber stamped without proper review.

67

Unfortunately, we don’t have such a policy, very similarly to how we lack even more basic policies (people are complaining about that these days in this thread). However, in my experience, if 1 person is able to verify the change is working with the hardware, or even if none, and the changes look good and software at least compiles and is able to not crash for a --help argument, it should be good to go.

For the 1st PR you linked, getting a :+1: from that person who requested the feature would greatly benefit, after most of the review is done.

1 Like
68

But, for changes such as in the 2nd PR, testing that this doesn’t break a setup for someone else is crucial. So that would require an approval from someone with that hardware available.

69

A PR rewriting i2pd nixos service is fixing its fundamental functionality is in:

The quality of writing seems very good to me - impressive for a first time contribution. I have little to no experience with freeformed submodules, and little experience with modules with so many options, so help will be appreciated.

70

nixos/generations-dir: Improve symlinking in `/boot` by Majiir · Pull Request #198728 · NixOS/nixpkgs · GitHub has been “LGTM” for almost a year. I think it’s stuck because not many reviewers run the relevant configuration to test it. (It can be tested on practically any hardware, but almost nobody runs generations-dir because there are better boot loader integrations for most systems.)

I’ve run this patch on one of my systems for at least as long as the PR has been up, and it’s been smooth sailing.

2 Likes
71

~~https://github.com/NixOS/nixpkgs/pull/249260~~

72

This PR for msodbcsql18 has been languishing for almost a year, and given that OpenSSL 1.1.1 is now EOL, It would be great to get an ODBC SQL Server driver that can support OpenSSL 3.0

73

Anyone knows what to do when the git repo doesn’t have any tags, and we want to build from source?

74

Haven’t looked at the PR, but in general, the fact there are no Git tags should have no effect on the decision whether to build from source or not. In general, building from source is preferred, with or without Git tags.

In case Git tags are missing, but there are version strings staggered in the source, it’d be nice to ping upstream about it, and use an explicit git revision in the meantime. Put a good comment near where the commit hash revision is used (instead of a rev = version or alike). Use the revision that marked the bump of the version string in the source.

2 Likes