python312Packages.premailer: drop nose dependency#330702
python312Packages.premailer: drop nose dependency#330702Sigmanificient wants to merge 1 commit intoNixOS:masterfrom
Conversation
emilazy
left a comment
There was a problem hiding this comment.
The disabled definition can be dropped.
Would you be up for sourcing this from GitHub instead and applying peterbe/premailer#288 to get the test suite to run? You can use a /compare/‹base revision›…‹last PR commit›.patch URL to get a reasonably stable patch without having to fetch every single commit individually, or just vendor the /pull/‹number›.patch file in Nixpkgs.
This is an unambiguous improvement though, so I won’t block it if you don’t feel like doing that.
|
Missing |
60fab22 to
fc7cde6
Compare
fc7cde6 to
0aa752d
Compare
|
The tests don’t pass even on Python 3.11: |
This is due to lxml having a parser difference(see reported bugs peterbe/premailer#72 and peterbe/premailer#294) in version 5.0.0 that breaks premailer. I'm not 100% sure why it does this, but I've confirmed this is the issue(pinning I'm not sure how to resolve this, but this is definitely the issue. |
|
If it’s been broken for this long, then perhaps we should simply drop it. The only user in‐tree is |
|
I think it'll be best to drop, since the project seems inactive and there are multiple PRs trying to fix support for py3.10+ which haven't been merged, and no PRs attempting to fix the lxml issue, which just creates a hassle moving forward. Will submit a new PR dropping the package. |
|
Closing as #348580 was merged. Thank you for your work on this! |
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.