[24.11] pyfa: 2.60.2 -> 2.61.3 #385471
[24.11] pyfa: 2.60.2 -> 2.61.3 #385471
Conversation
github-actions
bot
added
the
8.has: maintainer-list (update)
github-actions
bot
added
10.rebuild-darwin: 0
|
From what I can tell this looks good, but I have no experience with backports |
|
|
I think this is ready to merge, unless a backport needs something else that i dont know about. |
| cat > setup.py <<EOF | ||
| from setuptools import setup | ||
| setup( | ||
| name = "${pname}", | ||
| version = "${version}", | ||
| scripts = ["pyfa.py"], | ||
| packages = setuptools.find_packages(), | ||
| ) | ||
| EOF |
There was a problem hiding this comment.
Non-blocking: You could use writeText here.
| cat > setup.py <<EOF | ||
| from setuptools import setup | ||
| setup( | ||
| name = "${pname}", |
There was a problem hiding this comment.
Non-blocking: If you inline pname here, you can remove the rec keyword. (rec can lead to confusing errors so IIRC we try to avoid it, although I don't see this documented anywhere...)
There was a problem hiding this comment.
It's showing me an error in another file when trying to build the package. I think pname is used somewhere else?
There was a problem hiding this comment.
if that is the case, reading the Nix documentation leads me to believe that there's no way around rec.
There was a problem hiding this comment.
I think while I generally agree we should do best practice a backport PR is probably not the best time to fix those issues. I think finalAttrs is currently not supported for python packages otherwise we would have gone with that
There was a problem hiding this comment.
I agree, backport PRs are not the place to change this kind of stuff.
Backports are purely meant for porting changes made on unstable back to stable and similar actions.
On top of that, the creator of this PR didnt make this package, doesnt maintain it, said themselves that they arent well versed with git, so lets just not do this here and now.
There was a problem hiding this comment.
Oh sorry, I didn't realize this was a Nixpkgs backport, I thought it was a pyfa backport. Nevermind!
wegank
added
the
12.approvals: 1
ToasterUwU
added
12.approvals: 3+
wegank
added
12.approvals: 1
ToasterUwU
added
12.approvals: 3+
wegank
added
12.approvals: 1
|
gotta love wegank just refusing to see the actual 3 approvals this has... There also is a new pyfa version, but i wanna wait for this to be merged first |
@ToasterUwU feel free to update it upstream, I can backport it again when I have the time, it's good practice. :) |
Original PRs in main: #356622 #364987
My lack of foundational git knowledge is showing, hence the need for a new PR. Hope it didn't bombard everyone's inboxes.
Added commit ca3f599 to the branch, so hopefully the eval will pass.
cc: @ToasterUwU @Daholli @paschoal (do I not have permissions to add reviewers?)
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.