-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
All: delete outdated deprecations #16330
Conversation
33bd0c0
to
0468c5d
Compare
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.
Java 👍
I love that you keep doing these! Have you got the process fully automated by now?
The initial deletion of the predicates has been automated the entire time. |
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.
Ruby 👍
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.
Python 👍
@@ -0,0 +1,4 @@ | |||
--- | |||
category: minorAnalysis |
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.
Shouldn't this be breaking
?
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.
Probably this has been discussed in the past, sorry if that's the case.
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.
It is technically a breaking change to delete code. However, it's code that has been deprecated for more than a year, so external users have had plenty of to migrate.
So I still think it's a minor change.
I couldn't find a discussion in the past about this.
But all previous change-notes when deleting deprecations have been minorAnalysis
. (Since the first deprecation deletion PR).
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 don't think the amount of time the thing has been deprecated should influence whether or not this is a breaking change. A breaking change is a breaking change.
Obviously, this breaking change is totally justified, but I still think stuff like this should be marked as a breaking change going forward
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've changed it to breaking
.
I usually look at the previous PR when making the next one, so I should mark these as breaking
in the future.
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.
C++ 👍
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.
C# 👍
800d754
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.
Go 👍🏻
Similar to last time: #15393
Not that much this time.