-
Notifications
You must be signed in to change notification settings - Fork 824
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
API Update deprecations #10525
API Update deprecations #10525
Conversation
5a505ae
to
0c8782a
Compare
56b9647
to
9f89241
Compare
Why did you do #10530 as a separate PR? And why then include it in this PR? That's confusing. |
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.
There's at least one non-standard deprecation that hasn't been touched: Module::getCIConfig()
has the wrong version number, is missing a deprecation message in the phpdoc, and has an unhelpful message in the notice.
There are also bound to be cases which I missed where the phpdoc is missing the deprecation message. It needs to be in the notice and in the phpdoc.
I'm guessing based on what I have seen that you just ran some code writer over this and didn't look too closely at the output?
cf46483
to
eeaeed8
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.
There are some cases where the "Use [something else] instead" message is missing the word "instead" - but that doesn't materially change the message so asking you to change that would be nitpicking at this point - I'm just pointing it out to satisfy my brain but no change is required for that.
eeaeed8
to
b8e1ba8
Compare
b8e1ba8
to
9c453ab
Compare
Blocked by #10530
Issue #10500
There are 3 commits in this (will be only 2 after another PR is merged):