Problem/Motivation

Part of #1025928-18: [meta] Remove the last $op params from our hooks

There's few functions still using $op as argument (except controllers) in public methods (API)

Steps to reproduce

$ git grep '$op,'|grep public and git grep '$op)'|grep public

Proposed resolution

Rename it to $operation

Remaining tasks

patch/review/commit

User interface changes

no

API changes

no, just rename

Data model changes

no

Release notes snippet

Issue fork drupal-3353545

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.

andypost’s picture

Status: Active » Needs review
StatusFileSize
new10.24 KB

Remaining ones are in controllers and tests

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Don't mind marking this but it's going to need a reroll if the first ticket gets in. So probably should be postponed.

spokje’s picture

Status: Reviewed & tested by the community » Needs work

Needs a reroll, unsure why TestBot didn't kick this back to NR.

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new10.21 KB

re-roll, context changes in user.module

spokje’s picture

Status: Needs review » Reviewed & tested by the community

Rerolls on RTBC issues can go straight back to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs work

Needs another re-roll.

spokje’s picture

StatusFileSize
new6.05 KB

Reroll

spokje’s picture

Status: Needs work » Reviewed & tested by the community
andypost’s picture

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Status: Reviewed & tested by the community » Needs review

Still lots of $op arguments, at least some of these are considered API?

Regex for this search was function.*\$op\W.

composer/Plugin/Scaffold/ScaffoldFileInfo.php:  public function __construct(ScaffoldFilePath $destination, OperationInterface $op) {
core/lib/Drupal/Core/Config/ConfigImporterEvent.php:  public function getChangelist($op = NULL, $collection = StorageInterface::DEFAULT_COLLECTION) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function checkOp($collection, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function importConfig($collection, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function importInvokeOwner($collection, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function processConfiguration($collection, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function processExtension($type, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function setProcessedConfiguration($collection, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  protected function setProcessedExtension($type, $op, $name) {
core/lib/Drupal/Core/Config/ConfigImporter.php:  public function getExtensionChangelist($type, $op = NULL) {
core/lib/Drupal/Core/Config/StorageComparerInterface.php:  public function getChangelist($op = NULL, $collection = StorageInterface::DEFAULT_COLLECTION);
core/lib/Drupal/Core/Config/StorageComparer.php:  protected function addChangeList($collection, $op, array $changes, array $sort_order = NULL) {
core/lib/Drupal/Core/Config/StorageComparer.php:  protected function removeFromChangelist($collection, $op, $name) {
core/lib/Drupal/Core/Config/StorageComparer.php:  public function getChangelist($op = NULL, $collection = StorageInterface::DEFAULT_COLLECTION) {
core/modules/block/src/Controller/BlockController.php:  public function performOperation(BlockInterface $block, $op) {
core/modules/file/tests/file_test/file_test.module:function file_test_get_calls($op) {
core/modules/file/tests/file_test/file_test.module:function _file_test_get_return($op) {
core/modules/file/tests/file_test/file_test.module:function _file_test_log_call($op, $args) {
core/modules/file/tests/file_test/file_test.module:function file_test_set_return($op, $value) {
core/modules/search/src/Controller/SearchController.php:  public function performOperation(SearchPageInterface $search_page, $op) {
core/modules/system/src/Controller/DbUpdateController.php:  public function handle($op, Request $request) {
core/modules/system/tests/modules/image_test/src/Plugin/ImageToolkit/TestToolkit.php:  protected function logCall($op, $args) {
core/modules/system/tests/src/Functional/Entity/Traits/EntityDefinitionTestTrait.php:  protected function doEntityUpdate($op, $entity_type_id) {
core/modules/system/tests/src/Functional/Entity/Traits/EntityDefinitionTestTrait.php:  protected function doFieldUpdate($op, $storage_definition = NULL, $original_storage_definition = NULL) {
core/modules/user/tests/src/Kernel/UserMailNotifyTest.php:  public function testUserMailsNotSent($op) {
core/modules/user/tests/src/Kernel/UserMailNotifyTest.php:  public function testUserMailsSent($op, array $mail_keys) {
core/modules/views_ui/src/Controller/ViewsUIController.php:  public function ajaxOperation(ViewEntityInterface $view, $op, Request $request) {
core/tests/Drupal/Tests/Core/Access/AccessResultTest.php:  public function testAndOrCacheabilityPropagation(AccessResultInterface $first, $op, AccessResultInterface $second, $implements_cacheable_dependency_interface, $is_cacheable) {
smustgrave’s picture

Status: Needs review » Needs work

For #12

urvashi_vora’s picture

I will provide a patch soon

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

urvashi_vora’s picture

Status: Needs work » Needs review
StatusFileSize
new39.03 KB
new33.63 KB

Please review this patch. Replaced $op with $operation as per #12. I excluded controllers to replace $op with $operation.

Thanks

elber’s picture

Status: Needs review » Needs work

tests are failling

elber’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Needs work

How come the change to ScaffoldFileInfo.php was reverted?

elber’s picture

Status: Needs work » Needs review
elber’s picture

Hi I renamed the $op arguments in ScaffoldFileInfo.php to $operation, please revise

smustgrave’s picture

Status: Needs review » Needs work
StatusFileSize
new76.51 KB

Using the regex from #12 there are still some instances but just a few

1

elber’s picture

Status: Needs work » Needs review
andypost’s picture

Status: Needs review » Needs work
andypost’s picture

Status: Needs work » Needs review
Related issues: +#3380781: Rename $op with BC usage in update.php

\Drupal\Core\Update\UpdateKernel::setupRequestMatch() requires to use $op so I reverted it - needs separate issue to deprecate it properly - #3380781: Rename $op with BC usage in update.php

andypost’s picture

Status: Needs review » Needs work

There's few hunks needs to be moved to separate issues

- local variables - to prevent mix with arguments
- routing - needs to provide BC for routing arguments

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.