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.
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.
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 from that person who requested the feature would greatly benefit, after most of the review is done.
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.
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.
I believe that particular check would have passed if ofborg had run it at the time. If rerun now, it will probably fail, since something recently broke libpostal on macOS. Hopefully, that’s fixed by libpostal: fix on darwin by Thra11 · Pull Request #269292 · NixOS/nixpkgs · GitHub, but since I don’t believe that that failure is in any way related to the original PR, I’d rather keep things simple and merge the original PR first and then apply the darwin fix on top of it.