Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mattc321 created an issue. See original summary.

catch’s picture

Version: 9.x-dev » 9.0.x-dev

The 9.0.x branch will open for development soon, and the placeholder 9.x branch should no longer be used. Only issues that require a new major version should be filed against 9.0.x (for example, removing deprecated code or updating dependency major versions). New developments and disruptive changes that are allowed in a minor version should be filed against 8.9.x, and significant new features will be moved to 9.1.x at committer discretion. For more information see the Allowed changes during the Drupal 8 and 9 release cycles and the Drupal 9.0.0 release plan.

jibran’s picture

Title: Cleanup ViewsData::get(NULL) » Trigger deprecated warning for ViewsData::get(NULL)
Version: 9.0.x-dev » 8.9.x-dev
Status: Closed (duplicate) » Active

Let's add a deprecated warning.

kim.pepper’s picture

Status: Active » Needs review
Issue tags: +@deprecated
FileSize
1.48 KB

Adds a deprecation @trigger_error.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good to me, thanks!

+++ b/core/modules/views/src/ViewsData.php
@@ -137,12 +137,14 @@ public function getAll() {
+      @trigger_error('Calling get() without the $key argument is deprecated in drupal:8.8.0 and is required in drupal:9.0.0. See https://www.drupal.org/node/3090442', E_USER_DEPRECATED);

+++ b/core/modules/views/tests/src/Unit/ViewsDataTest.php
@@ -642,6 +642,9 @@ public function testCacheCallsWithoutWarmCacheAndGetMultipleTables() {
+   * @expectedDeprecation Calling get() without the $key argument is deprecated in drupal:8.8.0 and is required in drupal:9.0.0. See https://www.drupal.org/node/3090442

8.8 or 8.2?

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2723553-5.patch, failed testing. View results

kim.pepper’s picture

Status: Needs work » Needs review
FileSize
2.52 KB
2.64 KB

Removed a couple of usages.

Status: Needs review » Needs work

The last submitted patch, 8: 2723553-8.patch, failed testing. View results

kim.pepper’s picture

Status: Needs work » Needs review
FileSize
3.11 KB
608 bytes

Still some fails, but this should fix some too.

kim.pepper’s picture

Found another one.

The last submitted patch, 10: 2723553-10.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 11: 2723553-11.patch, failed testing. View results

jibran’s picture

Status: Needs work » Needs review
FileSize
694 bytes
4.53 KB

This should be green.

kim.pepper’s picture

Looks good to me! +1 to rtbc

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, for the review. Indeed it is RTBC.

Dinesh18’s picture

#14 looks good to me. +1 to RTBC

  • catch committed 9ac5c2b on 9.0.x
    Issue #2723553 by kim.pepper, jibran, mattc321: Trigger deprecated...

  • catch committed 2dcc847 on 8.9.x
    Issue #2723553 by kim.pepper, jibran, mattc321: Trigger deprecated...
catch’s picture

Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 9ac5c2b and pushed to 9.0.x, cherry-picked to 8.9.x and 8.8.x. Thanks!

  • catch committed 3c48ad8 on 8.8.x
    Issue #2723553 by kim.pepper, jibran, mattc321: Trigger deprecated...
jibran’s picture

Who added @mattc321 to the credit list. Did he do something here?

jibran’s picture

I was unable to find any proof of contribution form @mattc321 so removing @mattc321 for now. Feel free to add @mattc321 back if you have more info.

-Thanks
Jibran

catch’s picture

@jibran mattc321 opened the issue in the first place, we usually credit for that.

jibran’s picture

Sorry, I missed that.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.