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

Jump to bottom

djangorestframework-jwt: init at 1.11.0 #53698

Closed
wants to merge 3 commits into from
Closed

djangorestframework-jwt: init at 1.11.0 #53698

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 9, 2019 •
edited by ghost

Motivation for this change

I'm packaging this python library because I want to use nix-shell to manage python dependencies for a project.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ghost ghost requested a review from FRidh as a code owner January 9, 2019 13:47
dotlambda
dotlambda requested changes Jan 13, 2019
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Adding yourself to maintainer-list.nix should be a separate commit.

pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member

Btw, did you read https://nixos.org/nixpkgs/manual/#python?

@ghost
Copy link
Author

ghost commented Jan 18, 2019

@dotlambda Yes, I've read the python manual.

dotlambda
dotlambda requested changes Jan 18, 2019
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Why are you adding django-rest-auth without referencing it in python-packages.nix?

Also, please make sure the commit which adds you to the maintainer list is the first one.

maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/django-rest-auth/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/djangorestframework-jwt/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/django-rest-auth/default.nix Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Jan 18, 2019

@dotlambda I accidentally added the django-rest-auth module. Can I submit more than 1 module at once?

@dotlambda
Copy link
Member

@dotlambda I accidentally added the django-rest-auth module. Can I submit more than 1 module at once?

If it makes sense to, e.g. if these are all required for a certain package you're packaging, you should put these commits in a single PR.

@ghost
Copy link
Author

ghost commented Jan 20, 2019

I'll close this and open a second PR later on with all the other modules I'll need.

@ghost ghost closed this Jan 20, 2019
@dotlambda
Copy link
Member

Please ping me when you did so.

@ghost
Copy link
Author

ghost commented Jan 21, 2019

@dotlambda I have a package that doesn't include tests in the pypi release, is it a better practice to package the testless pypi or the github release that includes tests?

@dotlambda
Copy link
Member

If you plan on actively maintaining it, I think there's no harm in fetching from GitHub. Otherwise, PyPI is preferred because it allows for auto-upgrades. Best would be to file an upstream PR or issue so they include the tests in their PyPI tarball.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dotlambda @GrahamcOfBorg