Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add podgrab package and module #119443

Merged
merged 4 commits into from Apr 25, 2021
Merged

Add podgrab package and module #119443

merged 4 commits into from Apr 25, 2021

Conversation

ambroisie
Copy link
Contributor

Motivation for this change

Closes #117284.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ambroisie
Copy link
Contributor Author

I opened an issue upstream about changing the webui's port, given that 8080 does not seem like the best choice for a default value... If the issues gets resolved I will update the module to be able to change the port.

Also, I wasn't sure how to test the basic authentication when passwordFile is set. If there is an easy solution, I'll add it to the tests.

pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@lukegb lukegb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
pkgs/servers/misc/podgrab/default.nix Show resolved Hide resolved
pkgs/servers/misc/podgrab/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left a few comments on the module that I hope you will find helpful. Please let me know if you have any questions, etc...

nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/podgrab.nix Outdated Show resolved Hide resolved
@ambroisie ambroisie force-pushed the add-podgrab branch 2 times, most recently from e91996a to 351544d Compare April 15, 2021 16:23
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module and test look good 👍

If interested you can list yourself as maintainer of the module too.
I think we're supposed to list maintainers on tests, so maybe consider adding yourself there if you don't mind.

Thanks!

@ambroisie
Copy link
Contributor Author

Ah true I forgot to do that, will amend shortly.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

package LGTM

@ambroisie
Copy link
Contributor Author

Friendly ping about this MR :-)

@lukegb lukegb merged commit ed83f64 into NixOS:master Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Podgrab self-hosted podcast manager package & module
5 participants