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

Feature/upgrade django #127

Merged
merged 23 commits into from
Mar 15, 2023
Merged

Feature/upgrade django #127

merged 23 commits into from
Mar 15, 2023

Conversation

tijmenbaarda
Copy link
Contributor

@tijmenbaarda tijmenbaarda commented Oct 17, 2022

Upgrade to Django 4.1 and djangorestframework 3.14 (the latest versions). I did not stop at Django 3.2 (LTS) because the next version (4.2) will be an LTS and will still support Python 3.8. I cleaned up the code here and there in files that I was changing. Closes #119.

Copy link
Contributor

@jgonggrijp jgonggrijp left a comment

Choose a reason for hiding this comment

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

I was still running PostgreSQL 9.6 on my work machine, so I had to upgrade that first, but then the unittests passed. That's a good sign!

I also tried python manage.py runserver. That seems to run without issues, however there appears to be a subtle breaking change in the implementation of the {{ prefetched_* | escapejs }} variable interpolations in vre/templates/vre/base.html on lines 62 and 63. It now outputs a string that starts with the character b, which JSON.parse (rightly) rejects. Would be great if you could fix that, and then double-check again that the VRE runs in the browser without any errors in the console.

You seem to have an aggressive autoformatter, which (in my humble opinion) makes some odd choices when it comes to wrapping long lines. I guess that is a matter of taste; I will appreciate it if you align it closer to the idea behind 1TBS, but I will not insist. However, if possible, please tell it to only reformat lines that you changed.

Other than that, I'm happy with what I'm reading!

tijmenbaarda and others added 4 commits March 15, 2023 16:35
os import was removed for upgrade to Django 4.1 which uses pathlib
but we need it again because one of the hotfixes uses it
@tijmenbaarda tijmenbaarda merged commit 926ef59 into develop Mar 15, 2023
@tijmenbaarda tijmenbaarda deleted the feature/upgrade-django branch March 15, 2023 16:04
jgonggrijp added a commit that referenced this pull request Mar 20, 2023
I forgot to document this, and the updates to the requirements were
skipped in #127 as a result.
jgonggrijp added a commit that referenced this pull request Mar 20, 2023
I forgot to document this, and the updates to the requirements were
skipped in #127 as a result.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade all the dependencies
2 participants