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

python: django-compat & django-hijack tests (also bump django-hijack) #26166

Merged
merged 2 commits into from
May 30, 2017

Conversation

risicle
Copy link
Contributor

@risicle risicle commented May 28, 2017

Motivation for this change

Ah - seems we were both tinkering with this one at the same time.

Both django-compat and django-hijack's (same author) tests are run using a custom runtests.py script, which isn't included in the sdist packages on pypi. So I fixed the two packages by pulling the source from github tags instead and included a custom checkPhase.

While I was at it I also bumped django-hijack to the latest version and split it into a separate file.

I'm not totally sure as to the origin of django-hijack's django 1.9 requirement, but, well, the tests sure as hell pass with django 1.11.1. So I'm assuming it's safe to disregard that problem as ancient history...?

Added self as maintainer as I think I (or one of my old colleagues) added these packages in the first place.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@risicle, thanks for your PR! By analyzing the history of the files in this pull request, we identified @lsix and @FRidh to be potential reviewers.

@FRidh
Copy link
Member

FRidh commented May 28, 2017

Thanks @risicle . Could you keep in the comment I added? I think it is going to be relevant again at some point and I really don't want people to add overrides within the package set.

@FRidh FRidh self-assigned this May 28, 2017
@risicle
Copy link
Contributor Author

risicle commented May 28, 2017

Sure. I'll put it in the toplevel pythonPackages.nix for maximum visibility.

@risicle risicle force-pushed the django-compat-hijack-tests branch from a4ea2e7 to 17273a4 Compare May 28, 2017 11:24
@risicle
Copy link
Contributor Author

risicle commented May 28, 2017

Oh actually hold off on merging this would you, just want to check one more thing...

@risicle
Copy link
Contributor Author

risicle commented May 28, 2017

Yeah sorry, this is actually running the tests against the code in the source directory, not the installed package. I'll have to fix this.

also fix & enable tests, add self as maintainer
@risicle risicle force-pushed the django-compat-hijack-tests branch from 17273a4 to fa44b72 Compare May 28, 2017 13:44
@risicle
Copy link
Contributor Author

risicle commented May 28, 2017

Ok should be good to go - knock yourself out.

@FRidh FRidh requested a review from lsix May 29, 2017 08:03
@lsix
Copy link
Member

lsix commented May 30, 2017

Hi,

Sorry for late reply, I was away some day and still have some work to catch up ^_^

Looks good to me, and nox passes. I merge it right away.

@lsix lsix merged commit 61ce849 into NixOS:master May 30, 2017
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

4 participants