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

Issue fork drupal-3380781

Command icon 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

andypost created an issue. See original summary.

ankitsingh0188 made their first commit to this issue’s fork.

ankitsingh0188’s picture

Assigned: Unassigned » ankitsingh0188

ankitsingh0188’s picture

Assigned: ankitsingh0188 » Unassigned
Status: Active » Needs review
hetal.solanki’s picture

smustgrave’s picture

Status: Needs review » Needs work

find a way to throw deprecation message when $op is send

Doesn't appear to be addressed

ankitsingh0188’s picture

Status: Needs work » Needs review

@smustgrave - Please review my latest commit and let me know if there's anything else needs to be updated.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new1.99 KB

The 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.

ankitsingh0188’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

Think this should have a simple test for verifying the deprecation is triggered

ankitsingh0188’s picture

@smustgrave - I think there's no tests earlier for this file. Do we really need a test for this. Please suggest.

andypost’s picture

Not sure the test is required

ankitsingh0188’s picture

Status: Needs work » Needs review
smustgrave’s picture

Still think a test would be good, so will leave for the next reviewer.

smustgrave’s picture

So seems no one picked up after a month. Still maintain test coverage should be there has this seem like a unique deprecation.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new1.06 KB

The 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.

needs-review-queue-bot’s picture

Status: Needs work » Needs review

False positive

ankitsingh0188’s picture

Not sure the test is required. Let's leave it for the next reviewer.

smustgrave’s picture

Status: Needs review » Needs work

Well 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

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.