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

Add nix-shell setup for easy development #272

Closed
wants to merge 8 commits into from
Closed

Conversation

SEbbaDK
Copy link

@SEbbaDK SEbbaDK commented Oct 4, 2021

The shell specification uses the current requirements.txt and should add no maintenance.
It makes it very simple for nix users on linux/mac to get ready to work on stregsystemet via nix-shell.
We can hopefully remove the two custom packages later when @jbjjbjjbj upstreams the packages to nixpkgs

@SEbbaDK SEbbaDK changed the title Nix Add nix-shell setup for easy development Oct 4, 2021
shell.nix Outdated Show resolved Hide resolved
shell.nix Show resolved Hide resolved
Copy link
Member

@falkecarlsen falkecarlsen left a comment

Choose a reason for hiding this comment

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

I've heard it doesn't matter if the 'main' nix-file specifies py-version, but just for good order

django-debug-toolbar.nix Show resolved Hide resolved
django-select2.nix Show resolved Hide resolved
SEbbaDK and others added 2 commits October 4, 2021 20:13
This is because the nix python36Packages has some problems, so it's way easier to update this project to python 3.9 :^)
@falkecarlsen falkecarlsen marked this pull request as draft October 5, 2021 14:39
@SEbbaDK
Copy link
Author

SEbbaDK commented Oct 8, 2021

@falkecarlsen I made the shell show a clear warning about the python-version being used, so hopefully the mismatch isn't as big of a deal.

warning

@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.12%. Comparing base (17db314) to head (9a92e71).
Report is 176 commits behind head on next.

Additional details and impacted files
@@             Coverage Diff             @@
##             next     #272       +/-   ##
===========================================
- Coverage   82.48%   63.12%   -19.36%     
===========================================
  Files          30       30               
  Lines        2329     3051      +722     
  Branches      167      212       +45     
===========================================
+ Hits         1921     1926        +5     
- Misses        379     1061      +682     
- Partials       29       64       +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -14,7 +14,7 @@ jobs:
strategy:
max-parallel: 4
matrix:
python-version: [3.6, 3.7]
python-version: [3.6, 3.9]
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that this PR inadvertently also changes which next py version we're targeting. Since Django 2.2.17, there has been support for py39, so it's not a problem.

@krestenlaust krestenlaust requested a review from Mast3rwaf1z May 12, 2024 21:35
@Mast3rwaf1z
Copy link
Member

Mast3rwaf1z commented May 14, 2024

@krestenlaust it works with nixos-23.05, but later versions like 23.11 or unstable seems to have checks in place against the older version of django we use.

There seems to be a missing dependency to a module called django.utils.six, i don't know what this is.

Also this fork is very outdated

I would suggest that if we want a nix shell, i would prefer it to be written as a flake instead, and rewritten to work with the current system.

Edit: i do like how this is basing package versions on the requirements.txt tho

@krestenlaust
Copy link
Member

Thanks you for the insight @Mast3rwaf1z 👌

@soupglasses
Copy link
Member

Waiting for #409 so we dont need to package ancient versions of Python dependencies. 😎

@Mast3rwaf1z Mast3rwaf1z mentioned this pull request May 18, 2024
3 tasks
@Mast3rwaf1z
Copy link
Member

@krestenlaust with #459 merged, I think we should close this pr, it basically does the same with less flexibility

@krestenlaust
Copy link
Member

Fixed by #459

@krestenlaust krestenlaust removed the request for review from Mast3rwaf1z December 3, 2024 11:38
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.

7 participants