moonshine: 0.13.5 -> 0.14.0 - #546531
Conversation
Signed-off-by: Anish Pallati <i@anish.land>
Signed-off-by: Anish Pallati <i@anish.land>
✅
|
✅
|
neobrain
left a comment
There was a problem hiding this comment.
Thanks! Patch looks good and is working fine locally, but it's not a requirement to add yourself to the maintainer list. It would make more sense to add yourself after having done more substantial changes.
Also note there's a weekly auto-update on this package, which is generally a bit easier to review and merge than manual version bumps.
Thanks for the review! On the maintainer entry, the weekly auto-update is actually the main reason I'd like to be listed. I add myself as a maintainer to packages I use so that I can personally see to getting the r-ryantm PRs merged sooner. As far as I know there's no prerequisite of prior substantial changes for adopting a package in pkgs/by-name, but let me know if I've missed something. For context, I reviewed your original PR #544596 adding this package, so I'm not picking it up at random. If there's nothing you'd like changed in the patch itself, could you dismiss the review so this can get merged? |
|
Ah, I didn't notice the context of the original review. My personal opinion is that maintainers should at minimum test package binaries before submitting updates, which hasn't happened here. Adding new maintainers just so PRs land a day earlier doesn't seem very effective (especially if it comes at the cost of not testing anything). As you mention I don't think there are strictly documented requirements for becoming a maintainer, but since I'm not yet familiar with the soft conventions around nixpkgs maintainership I'll defer to my intuition there. My review doesn't block things though, so if other people are happy to add you to the list I don't mind. |
changelog
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.