-
Notifications
You must be signed in to change notification settings - Fork 194
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
Fix The DEFAULT_FILE_STORAGE setting is deprecated
warning
#408
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @KOliver94,
Foremost, thank you for your MR. I so happen to run into this same issue while maintaining django-silk
earlier this year, making a pass at this. I have gone through the mentioned tickets (#359 & #383), as well as cross-referencing this and the blocking MRs.
That said, this is good to go 🚀
storage = settings.DEFAULT_FILE_STORAGE | ||
storage = default_storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
Hey @frankwiles, Is there a chance to check my PR and get rid of the warning? Thanks! |
Hey @frankwiles, Django 5.1 is expected to be released in 3 months. The issue has been open for more than a year. There are 3 available PRs with fixes. Can you check #359, #383 or mine here please? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codingjoe I am really troubled with this warning, because I have -Werror
set. Everytime I access this in tests / development I see an error.
New release would be just amazing 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stepped down as a maintainer, but sure. This looks fine. Let's get it out the door.
Split from: #397
There are already two PRs for this (#359, #383) but as they are conflicting with master right now I included it here to avoid the anoying warnings.
Closes #359, closes #383, fixes #366