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
Packages for the python coreapi client #66391
Conversation
What are the changes to your fork of pythonPackages.coreschema? Typically we would keep those patches in the Nixpkgs repo if they are small. The packages look good overall. |
The fork deletes two files which aren't used but cause build errors. The errors occur when building the python3x version of the package, and without the files, builds succeed on all python versions (from what I can tell). |
That seems to me like something which could be done as part of the Nix package, maybe good to do that directly in the package so that we can use upstream directly? Although I see the upstream is archived, so maybe that's not useful; is there a fork of this package that is still actively developed/maintained? |
There are no forks that are maintained. All of them are behind or even with master. Should I patch up stream? |
I decided to patch upstream. The dependencies are still building, but I'm 99.99999999% confident it's fine. EDIT: build passes without a problem |
Yeah, that looks better, thanks for the change. This PR looks good to me now. |
Thanks for the review! |
To get the tests executing for the
The pypi package doesn't include the tests - they're present in the github archive though.
WFM macos 10.13 |
I'm a little confused over the necessity of The tests for
and
However these naturally include tests for the modules you've |
And for
and
|
@risicle thanks for the review! I've incorporated your changes, but I'm wondering, what's the purpose of moving the python module to The error was because in |
Oh, it's to "prove" that the tests are being run against the installed module, rather than the directory-local source dir.
Ah right yeah, sometimes people screw up the pypi packaging of a tarball, quite possible that's what's happened here. |
Is there a problem that I added an additional package that depends on the ones in the PR? I followed the same conventions as the previous ones. The new package is I tried to fetch the source from GitHub to run the tests, but I couldn't get past the modules attempting to access the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nix-review
passes on NixOS
diff LGTM, just one minor suggestion (doesn't change closure size so :/ )
No one will likely care much as long as you're a maintainer. And it's related to the others, should be fine. |
Please change into the tests directory then instead. That way the source won't get on |
This doesn't always work - sometimes tests expect the cwd to be the repo root for some reason. |
I can't find an example how to do that. Is it just changing the directory the tests run from? EDIT: |
you're over thinking it :) |
Thanks a lot, fixed :) |
I think this is ready to be merged, is there something left @FRidh ? |
…kage-coreapi Packages for the python coreapi client (cherry picked from commit e0fa73d)
Motivation for this change
I want to package my application with Nix and I need the coreapi package.
Things done
The coreapi python package depends on 2 packages that were not in Nix. I've packaged those as well. One of them had to be forked and updated. I've added myself as a maintainer to them.
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @