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

feat: support django 4.2 new STORAGES format #728

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

bellini666
Copy link
Contributor

Fix #716

@claudep
Copy link
Contributor

claudep commented Apr 3, 2023

Thanks! Would you also like to suggest a PR which adds Django 4.2 in the testing matrix?

@bellini666
Copy link
Contributor Author

Thanks! Would you also like to suggest a PR which adds Django 4.2 in the testing matrix?

done! I updated test.yaml and tox.ini, not sure if it is enough

@bellini666 bellini666 requested a review from claudep April 3, 2023 20:40
@codecov
Copy link

codecov bot commented Apr 3, 2023

Codecov Report

Merging #728 (71f3f44) into master (b5f4d95) will increase coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 71f3f44 differs from pull request most recent head 769b249. Consider uploading reports for the commit 769b249 to get more accurate results

@@            Coverage Diff             @@
##           master     #728      +/-   ##
==========================================
+ Coverage   73.70%   73.76%   +0.06%     
==========================================
  Files          30       30              
  Lines        1696     1700       +4     
==========================================
+ Hits         1250     1254       +4     
  Misses        446      446              
Impacted Files Coverage Δ
sorl/thumbnail/conf/defaults.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@claudep
Copy link
Contributor

claudep commented Apr 5, 2023

Any clue about the test failures?

@bellini666
Copy link
Contributor Author

Any clue about the test failures?

@claudep looking at the tests, they failed with ModuleNotFoundError: No module named 'django'.

Strange that I did not modify any behaviour there, I just added python 3.11 and django 4.2 to the test matrix.

I made one typo which I just pushed a fix, but I don't believe it would be that.

Either way, can you approve the tests to run again?

Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

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

Looks good now!

By the way, I think we should move the THUMBNAIL_STORAGE setting to a key from the STORAGES dict, so we can get the storage directly from the django.core.files.storage.storages structure. Would you like to continue working on this?

@claudep claudep merged commit 4e77896 into jazzband:master Apr 5, 2023
@bellini666
Copy link
Contributor Author

By the way, I think we should move the THUMBNAIL_STORAGE setting to a key from the STORAGES dict, so we can get the storage directly from the django.core.files.storage.storages structure. Would you like to continue working on this?

Sure!Can you open an issue for that and assign it to me, so that I don't forget it? =P

@claudep
Copy link
Contributor

claudep commented Apr 5, 2023

Done with #729, but I cannot assign you (don't ask me why, GitHub mystery).

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.

Support Django 4.2 - settings.DEFAULT_FILE_STORAGE is deprecated
2 participants