CommentFileSizeAuthor
#14 3097108-14.patch7.63 KBJeroenT
#5 interdiff.3097108.2-5.txt7.79 KBlongwave
#5 3097108-5.drupal.Remove-dblogmodule-BC-layers.patch11.49 KBlongwave
#2 3097108-2.drupal.Remove-dblogmodule-BC-layers.patch3.7 KBlongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Active » Needs review
FileSize
3097108-2.drupal.Remove-dblogmodule-BC-layers.patch3.7 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Parent issue: » #2716163: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 9 branch

Status: Needs review » Needs work

The last submitted patch, 2: 3097108-2.drupal.Remove-dblogmodule-BC-layers.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
3097108-5.drupal.Remove-dblogmodule-BC-layers.patch11.49 KB
interdiff.3097108.2-5.txt7.79 KB
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community
Related issues: +#3087644: Remove Drupal 8 updates up to and including 88**

It looks good to remove upgrades here to minify work in #3087644: Remove Drupal 8 updates up to and including 88**

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

This may need to wait on #3097661: No hook_update_last_removed() equivalent for post updates.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

In #3087644: Remove Drupal 8 updates up to and including 88** (which at least partially overlaps with this issue (this doesn't remove the post_update yet but it does remove its test), I suggested that adding an update function to clean the post update storage could be a follow-up, we could make that one a critical beta blocker, but the mentioned issue is already massive, and adding the API first will block a lot of BC removal issues.

That said, not sure about this, seems a bit strange to remove just the resave hook and one of the multiple update tests.. maybe get the other issue in first and then deal with the leftovers here like the presave hook here?

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented

The test tests both the post update hook and dblog_view_presave(). It perhaps should have been two test methods!

We could just remove the second half of the test and dblog_view_presave() here, and leave the rest to the update hooks issue?

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3097108-5.drupal.Remove-dblogmodule-BC-layers.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Needs review

Unrelated failure

Drupal\BuildTests\Framework\Tests\BuildTestTest::testPortMany
RuntimeException: Unable to start the web server.
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Assigned: Unassigned » JeroenT
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Issue tags: +ContributionWeekend2020
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
FileSize
3097108-14.patch7.63 KB

Patch no longer applied. Created a reroll.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Assigned: JeroenT » Unassigned

The patch in #14 still needs to remove the post_update dblog_post_update_convert_recent_messages_to_view but that's blocked on #3097661: No hook_update_last_removed() equivalent for post updates

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I believe you can simply assume that the post update is gone because we have no tests anymore that run it. And then we can remove them all in #3106666: Remove post updates added prior to 8.8.0

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

#16: great!

Manually tested: applied the patch and verified that no more @deprecated occurrences can be found in core/modules/dblog.

🚢

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed e809a80 and pushed to 9.0.x. Thanks!

diff --git a/core/modules/dblog/dblog.module b/core/modules/dblog/dblog.module
index 3687a73b14..8e6a49ad93 100644
--- a/core/modules/dblog/dblog.module
+++ b/core/modules/dblog/dblog.module
@@ -13,7 +13,6 @@
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
-use Drupal\views\ViewEntityInterface;
 use Drupal\views\ViewExecutable;
 
 /**

Fix unused use.

  • alexpott committed e809a80 on 9.0.x 
    Issue #3097108 by longwave, JeroenT, Berdir, andypost: Remove dblog....

Status: Fixed » Closed (fixed)

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