Skip to content

Add support for Django 5.2 #257

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.

Sign up for GitHub

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
Merged
merged 2 commits into from
May 6, 2025
Merged

Add support for Django 5.2 #257

merged 2 commits into from
May 6, 2025

Conversation

hinakhadim
Copy link
Contributor

@hinakhadim hinakhadim commented Apr 17, 2025 •
edited
Loading

Add Django 5.2 Support

This PR updates the codebase to be compatible with Django 5.2 while maintaining compatibility with Django 4.2.

Changes Made:

  • Update ci and tox files
  • Ran django-upgrade to apply necessary syntax updates for Django 5.2.
  • Run tests using tox command and resolved the warning
  • Ensured all modifications remain backward-compatible with Django 4.2.

django-upgrade usage:

I ran command git ls-files -z -- '*.py' | xargs -0r django-upgrade --target-version 5.2

@hinakhadim
Copy link
Contributor Author

Updated requirements to make the CI tests pass. Otherwise, following error is generating:

removing build/bdist.linux-x86_64/wheel
quality: commands[3]> twine check 'dist/*'
Checking dist/openedx_django_pyfs-3.8.0-py3-none-any.whl: ERROR    InvalidDistribution: Metadata is missing required fields: Name,        
         Version.                                                               
         Make sure the distribution includes the files where those fields are   
         specified, and is using a supported Metadata-Version: 1.0, 1.1, 1.2,   
         2.0, 2.1, 2.2, 2.3.                                                    
quality: exit 1 (0.29 seconds) /home/runner/work/django-pyfs/django-pyfs> twine check 'dist/*' pid=2216
  quality: FAIL code 1 (34.63=setup[26.71]+cmd[7.12,0.12,0.38,0.29] seconds)
  evaluation failed :( (34.78 seconds)

@hinakhadim hinakhadim force-pushed the django52support branch 2 times, most recently from 5c75b8b to 7fe1cd5 Compare April 18, 2025 11:37
awais786
awais786 reviewed Apr 21, 2025
View reviewed changes
example/example/settings.py
@@ -46,10 +46,6 @@
# to load the internationalization machinery.
USE_I18N = True

# If you set this to False, Django will not format dates, numbers and
# calendars according to the current locale.
USE_L10N = True
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to find out how it will effect prod ?

Copy link
Contributor Author

@hinakhadim hinakhadim Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We currently have USE_L10N = True in this repository. Since this setting is enabled by default in Django 4.2 and removed entirely in Django 5.2, it is safe to remove without causing any issues in production.

P.S. As @irtazaakram shared this URL, there is no instance of USE_L10N = False in Open edX, so removing it aligns with the existing usage across the platform.

@awais786
Copy link
Contributor

@feanil please review.

@feanil
temp: Re-build with a newer pip-compile.
c16b857 
The older version was using full requirements paths in its comments
which is not usefule.

Also update tox.ini to run tests on Django 5.2 and drop testing on 5.1
@feanil feanil merged commit 339509c into openedx:master May 6, 2025
8 checks passed
@codecov Codecov
Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (2a4560c) to head (c16b857).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@      Coverage Diff      @@
##   master   #257   +/-   ##
=============================
=============================

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awais786 awais786 awais786 approved these changes

@feanil feanil feanil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Django 5.2 support to django-pyfs
3 participants
@hinakhadim @awais786 @feanil