- Issue created by @dww
- 🇫🇷France andypost
@dww can you move existing MR here https://git.drupalcode.org/project/drupal/-/merge_requests/10461
- 🇺🇸United States dww
@andypost: Not sure what you're requesting in #3. That MR is for a set of changes to completely remove all this code. Instead, it'll probably be more simple to start over from scratch here, since there's now a different scope and approach needed for this issue.
But if I'm misunderstanding, please let me know.
Thanks!
-Derek - 🇺🇸United States dww
Opened a CR for this (to get a valid NID for deprecation messages) and started deprecating everything in the IS. Pushed commits for
authorize.phpandsystem_authorized_*(). Opened an MR. Still incomplete, so NW, but wanted to at least get this started and to see if the bot explodes in any unexpected ways. 😅 - 🇺🇸United States dww
I asked in #core-development in Slack, but x-posting here:
core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.phpis now failing since it's triggering deprecations. Can I completely remove that test as part of this issue, or do I need to convert it to expect deprecations, etc? - 🇬🇧United Kingdom catch
I think @group legacy will allow the test to pass with deprecations without changing anything else about it. We have to use that on update test for similar reasons.
- 🇫🇷France andypost
Maybe deprecation of hooks could be done in module handler? gonna dig it
- 🇺🇸United States dww
@andypost, see:
https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... →I’m assigning myself, since I’ve got more of this done locally. Stay tuned.
- 🇺🇸United States dww
Okay, cool, that's working now.
Adding a TODO list under remaining tasks, and crossing out what's already been pushed...
- 🇺🇸United States dww
@andypost: Argh, what are you doing? I assigned this to myself and am actively working on it.
- 🇺🇸United States dww
Added an item to API changes for:
- Every method in
core/modules/update/update.authorize.inc
Updating remaining tasks to cross off what's done. Almost there...
- Every method in
- 🇺🇸United States dww
Added another API change point for:
- Every method in
core/modules/update/update.manager.inc
Everything is now done. Pipeline is green. Simplified remaining tasks.
This is now ready for review (both the MR and the CR). Unassigning.
Thanks!
-Derek - Every method in
- 🇫🇷France andypost
I'd like to set RTBC but probably exception classes needs constructor to throw deprecation...
but they are inherited so both will throw - 🇺🇸United States dww
Re:
except probably exceptions need constructor to throw deprecations
From @catch via Slack:
https://drupal.slack.com/archives/C079NQPQUEN/p1741811869453559?thread_t...
catch: I don't think we have anything explicit for deprecating exceptions, but given these aren't used anywhere outside the system we're deprecating I think just @trigger_error() is fine.
dww: But then I have to add__construct()methods. 😅 Can I just do the annotation, instead?
catch: Sorry that's what I meant... - 🇫🇷France andypost
Checked usage in contrib and it could be found in DB-mocks of D7 http://codcontrib.hank.vps-private.net/search?text=UpdaterFileTransferEx...
RTBC as both code and CR looks ready
- 🇬🇧United Kingdom catch
Given zero actual usages of those exceptions I think it's fine to add just the annotation. Everything else looks good. Ideally phpstan would ignore deprecated usages inside deprecated code but either it doesn't or it's not perfect.
Committed/pushed to 11.x, thanks!
