-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Reintroduce warnings postponed in 6.0 #7637
Reintroduce warnings postponed in 6.0 #7637
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.
Thanks a lot @mcsitter for getting to this.
I've dug around the Git history to find the original deprecation messages for those entries, please see my comments. 👍
I don't understand our deprecation plans here - all deprecations are errors now, even new deprecations we're introducing? In other words, there is no phase where new deprecations are warnings rather than errors? |
Yeah I didn't mention, but one of the first things we should do is #5585 and turn back |
Co-authored-by: Hugo van Kemenade <[email protected]>
After #7660 gets merged we can rebase this one. 👍 |
Will do! I never rebased a branch in a PR. I think I have to 1. pull from upstream master with the rebase flag. Once I learned to 2. deal with merge conflicts, I will 3. push the changes to my fork. I think a rebase can rewrite history, so i am trying to be careful. |
That's pretty much it, yes. Let us know if you run into trouble! Thanks!
You are right, a rebase normally rewrites history, but it is only on your fork so it is alright. 👍 |
Rebase done. Thank you for your support! Tests are also passing. This is really delightful 😄 |
Looks great! Thanks a lot @mcsitter! |
I came across the comment
# Uncomment this after 6.0 release (#7361)
. Here is my PR for this.Still working on the changelog text. In
deprecated.py
there are explanations for the deprecations already. Should I just copy them and shorten where it makes sense?closes #7361