Line tabledrag_example/tabledrag_example.install

Call to deprecated function db_drop_table(): in drupal:8.0.0 and will be removed from drupal:9.0.0. Instead, get a database connection injected into your service from the container, get its schema driver, and call dropTable() on it.

CommentFileSizeAuthor
#15 3072418-15.patch476 bytesandypost
#4 3072418-drop-table-dep-4.patch506 bytesSerShevchyk
#2 3072418-drop-table-dep.patch690 bytesSerShevchyk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Shefarik created an issue. See original summary.

SerShevchyk’s picture

SerShevchyk
Primary language Ukrainian
Location Lutsk
CreditAttribution: SerShevchyk as a volunteer and at Drupal Ukraine Community commented
Status: Active » Needs review
FileSize
3072418-drop-table-dep.patch690 bytes
dpovshed’s picture

dpovshed CreditAttribution: dpovshed as a volunteer and at Drupal Ukraine Community commented
Status: Needs review » Needs work

I would suggest using the same syntax and variable names as in the tabledrag_example_install() function, i.e.

  /* @var $connection \Drupal\Core\Database\Connection */
  $connection = \Drupal::database();
  ...
SerShevchyk’s picture

SerShevchyk
Primary language Ukrainian
Location Lutsk
CreditAttribution: SerShevchyk as a volunteer and at Drupal Ukraine Community commented
FileSize
3072418-drop-table-dep-4.patch506 bytes
SerShevchyk’s picture

SerShevchyk
Primary language Ukrainian
Location Lutsk
CreditAttribution: SerShevchyk as a volunteer and at Drupal Ukraine Community commented
Status: Needs work » Needs review
dpovshed’s picture

dpovshed CreditAttribution: dpovshed as a volunteer and at Drupal Ukraine Community commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Related issues: +#3072431: \Drupal database use dependency injection instead
lyalyuk’s picture

lyalyuk CreditAttribution: lyalyuk at Drupal Ukraine Community for Drupal Ukraine Community commented
Related issues: +#3072411: Fix tabledrag_example module to allow drupal-check run and convert tabledrag_example_data() usage

This issue is already fixed here https://www.drupal.org/project/examples/issues/3072411

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald at Dropsolid commented

@lyalyuk it's harder to review bigger patches that cover multiple deprecations. Do you coordinate/track the changes that happen in other tickets?

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald at Dropsolid commented

Crediting @dpovshed for the review

SerShevchyk’s picture

SerShevchyk
Primary language Ukrainian
Location Lutsk
CreditAttribution: SerShevchyk as a volunteer and at Drupal Ukraine Community commented
Assigned: SerShevchyk » Unassigned

unassigned by maintainer's request

lyalyuk’s picture

lyalyuk CreditAttribution: lyalyuk at Drupal Ukraine Community for Drupal Ukraine Community commented

@valthebald Yep, we were a little miscoordinated here but now everything is ok. My patch in the https://www.drupal.org/project/examples/issues/3072411 contains all our work on the `tabledrag_example`
The huge part of the patch is just moving function tabledrag_example_data into a static method in TableDragExampleResetForm. Cause drupal-check claims on module_load_include.
I think using module_load_include doesn't make a sense in the border of one module. And I suppose the static method will work faster.

valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald at Dropsolid commented

@lyalyuk awesome!

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

I think better commit per file/per case - this one perfectly separate since 8.0

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
3072418-15.patch476 bytes

The proper fix is like #3072411: Fix tabledrag_example module to allow drupal-check run and convert tabledrag_example_data() usage
So no reason for intermediate variable here, sounds like good enough to be separate

  • valthebald committed c9c9052 on 8.x-1.x 
    Issue #3072418 by Shefarik, andypost, dpovshed: Call to deprecated...
valthebald’s picture

valthebald
Location Sofia
CreditAttribution: valthebald at Dropsolid commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x-1.x, thank you all!

Status: Fixed » Closed (fixed)

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

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented
Issue tags: +Drupal 9 compatibility

Per a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.