Problem/Motivation
Follow-up to #3353545-26: Rename $op to $operation arguments
The script /update.php using \Drupal\Core\Update\UpdateKernel::setupRequestMatch() to provide $op operation for \Drupal\system\Controller\DbUpdateController::handle()
Proposed resolution
- provide both $op and $operation in UpdateKernel::setupRequestMatch()
- rename \DbUpdateController::handle() the $op argument to $operation
- find a way to throw deprecation message when $op is send
Remaining tasks
- patch and change record
- review
- commit
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 3380781-nr-bot.txt | 1.06 KB | needs-review-queue-bot |
| #9 | 3380781-nr-bot.txt | 1.99 KB | needs-review-queue-bot |
Issue fork drupal-3380781
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
ankitsingh0188Comment #5
ankitsingh0188Comment #6
hetal.solankiComment #7
smustgrave commentedDoesn't appear to be addressed
Comment #8
ankitsingh0188@smustgrave - Please review my latest commit and let me know if there's anything else needs to be updated.
Comment #9
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #10
ankitsingh0188Comment #11
smustgrave commentedThink this should have a simple test for verifying the deprecation is triggered
Comment #12
ankitsingh0188@smustgrave - I think there's no tests earlier for this file. Do we really need a test for this. Please suggest.
Comment #13
andypostNot sure the test is required
Comment #14
ankitsingh0188Comment #15
smustgrave commentedStill think a test would be good, so will leave for the next reviewer.
Comment #16
smustgrave commentedSo seems no one picked up after a month. Still maintain test coverage should be there has this seem like a unique deprecation.
Comment #17
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #18
needs-review-queue-bot commentedFalse positive
Comment #19
ankitsingh0188Not sure the test is required. Let's leave it for the next reviewer.
Comment #20
smustgrave commentedWell this issue has sat for almost 2 months but I don’t plan to mark without test coverage.
We add test coverage for other deprecations not sure how this should be exempt