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

deluge: shut up ngettext error on webui start #115055

Merged
merged 2 commits into from Mar 29, 2021
Merged

Conversation

peterhoeg
Copy link
Member

Motivation for this change

deluge would complain loudly on start. Also patch the logger on py38.

Noise from nixpkgs-fmt.

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.

@peterhoeg
Copy link
Member Author

We should do a manual backport to 20.09 when this is merged.

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 115055 at 566f173d run on x86_64-linux 1

1 package built:
1 suggestion:
  • warning: missing-patch-comment

    Please add a comment on the line above, explaining the purpose of this patch.
    Near pkgs/applications/networking/p2p/deluge/default.nix:24:5:

       |
    24 |     (fetchpatch {
       |     ^
    

@peterhoeg
Copy link
Member Author

Hey @rmcgibbo - as for the suggestion regd a patch comment, maybe the bot can check for a file name and adjust the message to something like this:

Please add a comment on the line above, explaining the purpose of this patch unless the file name is sufficiently informative.

sha256 = "sha256-slGMt2bgp36pjDztJUXFeZNbzdJsus0s9ARRD6IpNUw=";
name = "fix_ngettext_warning.patch";
})
] ++ lib.optional pythonPackages.isPy38 (fetchpatch {
Copy link
Member

Choose a reason for hiding this comment

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

Does this break anything on other python versions? If not we shouldn't apply this conditionally to prevent bitrot.

Copy link
Member Author

Choose a reason for hiding this comment

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

This patch is only needed for 3.8, but it will be part of the next deluge release, so we'll just drop the patches then.

Copy link
Member

Choose a reason for hiding this comment

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

If by then the default python version migrated to 3.9 this patch will rot here. If it breaks nothing on other versions we can apply it unconditional to definitely notice when it breaks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough. Fixed.

pkgs/applications/networking/p2p/deluge/default.nix Outdated Show resolved Hide resolved
@peterhoeg
Copy link
Member Author

@ofborg test deluge

Also patch the logger on py38.
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
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.

None yet

3 participants