Problem/Motivation

From #2743313-16: [policy, docs] Use post updates for empty 'clear the cache' updates

What about creating a patch as part of this issue to change the comments within all of our existing empty update hooks? Similar to what we have in block_content_update_8002()?

Proposed resolution

grep -rni 'function.*_update_\d' core
Use above command to find all the hook_update_N and then select empty hook_update_N used for cache clear only.

Remaining tasks

Find out all the empty hook_update_N used for cache clearing and add the following docs.

// Use of hook_post_update_NAME instead to clear the cache. 
// The use of hook_update_N to clear the cache has been deprecated
// see https://www.drupal.org/node/2960601 for more details.

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#22 docs-empty-hook_update_N-2960605-21.patch4.39 KBarunkumark
#21 interdiff_18_20.txt4.83 KBanmolgoyal74
#21 2960605-20.patch4.41 KBanmolgoyal74
#18 interdiff-14_18.txt6.29 KBsurbz
#18 2960605-18.patch4.4 KBsurbz
#14 interdiff_10_14.txt389 bytesanmolgoyal74
#14 add-docs-to-empty-hook-update-n-2960605-14.patch4.4 KBanmolgoyal74
#10 interdiff_8_10.txt3.73 KBanmolgoyal74
#10 interdiff_4_10.txt6.99 KBanmolgoyal74
#10 add-docs-to-empty-hook-update-n-2960605-10.patch4.96 KBanmolgoyal74
#8 add-docs-to-empty-hook-update-n-2960605-8.patch10.03 KBanmolgoyal74
#4 add-docs-to-empty-hook-update-n-2960605-4.patch13.29 KBmsankhala

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Issue summary: View changes
msankhala’s picture

msankhala commented
Issue summary: View changes
msankhala’s picture

msankhala commented
StatusFileSize
new add-docs-to-empty-hook-update-n-2960605-4.patch13.29 KB

Here is a patch.

msankhala’s picture

msankhala commented
Status: Active » Needs review
chanderbhushan’s picture

chanderbhushan commented

I have tested #4 is looks fine thanks

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work 
+++ b/core/modules/system/tests/modules/entity_test/update/status_report_8001.inc
@@ -10,4 +10,8 @@
 function entity_test_update_8001() {
   // Empty update, we just want to trigger an error in the status report.
+
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/entity_test/update/status_report_8002.inc
@@ -12,4 +12,8 @@
 function entity_test_update_8002() {
   // Empty update, we just want to trigger an update run.
+
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_0/update_test_0.install
@@ -9,16 +9,25 @@
 function update_test_0_update_8001() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_0_update_8002() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_0_update_8003() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_1/update_test_1.install
@@ -40,16 +40,25 @@ function update_test_1_update_dependencies() {
 function update_test_1_update_8001() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_1_update_8002() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_1_update_8003() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_2/update_test_2.install
@@ -38,16 +38,25 @@ function update_test_2_update_dependencies() {
 function update_test_2_update_8001() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_2_update_8002() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_2_update_8003() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_3/update_test_3.install
@@ -21,4 +21,7 @@ function update_test_3_update_dependencies() {
 function update_test_3_update_8001() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_failing/update_test_failing.install
@@ -19,4 +19,8 @@ function update_test_failing_update_8001() {
 function update_test_failing_update_8002() {
   // This hook won't ever run.
+
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_invalid_hook/update_test_invalid_hook.install
@@ -9,4 +9,7 @@
 function update_test_invalid_hook_update_8000() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.install
@@ -9,4 +9,7 @@
 function update_test_postupdate_update_8001() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

+++ b/core/modules/system/tests/modules/update_test_with_7x/update_test_with_7x.install
@@ -9,12 +9,18 @@
 function update_test_with_7x_update_7200() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.
...
 function update_test_with_7x_update_7201() {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

This is test code and this hook is not used to clear the cache so please remove this change.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
StatusFileSize
new add-docs-to-empty-hook-update-n-2960605-8.patch10.03 KB

did all the changes told by @jibran in #7.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Needs work

Thanks, for the changes. While updating the existing patch please always create an interdiff.

  1. +++ b/core/modules/system/tests/modules/entity_test/update/status_report_8001.inc
@@ -9,5 +9,5 @@
-  // Empty update, we just want to trigger an error in the status report.

+++ b/core/modules/system/tests/modules/entity_test/update/status_report_8002.inc
@@ -11,5 +11,5 @@
-  // Empty update, we just want to trigger an update run.

    Please restore this line. 

  2. +++ b/core/modules/system/tests/modules/entity_test/update/status_report_8001.inc
@@ -9,5 +9,5 @@
+  ¶

+++ b/core/modules/system/tests/modules/entity_test/update/status_report_8002.inc
@@ -11,5 +11,5 @@
+  ¶

+++ b/core/modules/system/tests/modules/update_test_0/update_test_0.install
@@ -9,16 +9,19 @@
+ ¶
...
+  ¶
...
+  ¶

+++ b/core/modules/system/tests/modules/update_test_1/update_test_1.install
@@ -40,16 +40,19 @@ function update_test_1_update_dependencies() {
+  ¶
...
+  ¶
...
+  ¶

+++ b/core/modules/system/tests/modules/update_test_2/update_test_2.install
@@ -38,16 +38,19 @@ function update_test_2_update_dependencies() {
+  ¶
...
+  ¶
...
+  ¶

+++ b/core/modules/system/tests/modules/update_test_3/update_test_3.install
@@ -21,4 +21,5 @@ function update_test_3_update_dependencies() {
+ ¶

+++ b/core/modules/system/tests/modules/update_test_failing/update_test_failing.install
@@ -19,4 +19,6 @@ function update_test_failing_update_8001() {
+
+  ¶

+++ b/core/modules/system/tests/modules/update_test_invalid_hook/update_test_invalid_hook.install
@@ -9,4 +9,5 @@
+  ¶

+++ b/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.install
@@ -9,4 +9,5 @@
+  ¶

+++ b/core/modules/system/tests/modules/update_test_with_7x/update_test_with_7x.install
@@ -9,12 +9,14 @@
+  ¶
...
+  ¶

    Please clean up the whitespaces.

anmolgoyal74’s picture

anmolgoyal74 commented
StatusFileSize
new add-docs-to-empty-hook-update-n-2960605-10.patch4.96 KB
new interdiff_4_10.txt6.99 KB
new interdiff_8_10.txt3.73 KB

Please review the patch.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
msankhala’s picture

msankhala commented
Status: Needs review » Reviewed & tested by the community

Patch #10 has all the fixes as per suggestion in #7 and #9 by @jibran. This patch applies cleanly.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Reviewed & tested by the community » Needs work

Thanks, for fixing the feedback. Just a minor observation.

+++ b/core/modules/system/tests/modules/entity_test/update/status_report_8002.inc
@@ -11,5 +11,5 @@
-  // Empty update, we just want to trigger an update run.
+  // Empty update, we just want to trigger an error in the status report.

Unrelated change please revert this.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
StatusFileSize
new add-docs-to-empty-hook-update-n-2960605-14.patch4.4 KB
new interdiff_10_14.txt389 bytes

Reverted as per suggestion in #13.

surbz’s picture

surbz commented

Patch #14 has all the fixes as per suggestion in #13 by @jibran. This patch applies cleanly.

surbz’s picture

surbz commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/image/image.install
@@ -67,4 +67,8 @@ function image_requirements($phase) {
+  // Use of hook_post_update_NAME instead to clear the cache.
+  // The use of hook_update_N to clear the cache has been deprecated
+  // see https://www.drupal.org/node/2960601 for more details.

Use of X instead is not correct grammar. It should be Use X instead. Plus functions should have () after them. Plus we have 80 characters before the comment line should break. Let's use them. So the comment should be:

  // Use hook_post_update_NAME() instead to clear the cache. The use of
  // hook_update_N() to clear the cache has been deprecated see
  // https://www.drupal.org/node/2960601 for more details.
surbz’s picture

surbz commented
Status: Needs work » Needs review
StatusFileSize
new 2960605-18.patch4.4 KB
new interdiff-14_18.txt6.29 KB

Thanks @alexpott for reviewing this i am making the fixes from #17 here.
Also attaching an interdiff.txt

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/image/image.install
@@ -67,4 +67,8 @@ function image_requirements($phase) {
+  // hook_update_N to clear the cache has been deprecated see

    Missing a () after hook_update_N. 

  2. +++ b/core/modules/system/system.install
@@ -1837,6 +1837,10 @@ function system_update_8200(&$sandbox) {
+  // Use hook_post_update_NAME() instead to clear the cache.The use
+  // of hook_update_N to clear the cache has been deprecated see

    Some of these are missing a space after cache. also the comment flow is not quite right... of can be on the previous line.

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Assigned: Unassigned » arunkumark
anmolgoyal74’s picture

anmolgoyal74 commented
Assigned: arunkumark » anmolgoyal74
Status: Needs work » Needs review
StatusFileSize
new 2960605-20.patch4.41 KB
new interdiff_18_20.txt4.83 KB

Thanks @alexpott for reviewing this. I'm making the fixes from #19 here.
Also attaching an interdiff.txt

arunkumark’s picture

arunkumark
Location Coimbatore
commented
Assigned: anmolgoyal74 » Unassigned
StatusFileSize
new docs-empty-hook_update_N-2960605-21.patch4.39 KB

Re-rolled the patch as per the comment on #19.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented

Tested #21 and it applies cleanly, also the #19 issues are fixed.

@arunkumark next time please provide interdiff so your patch can be easier to review.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Thanks, for the patch.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 0076c63 and pushed to 8.6.x. Thanks!

  • catch committed 0076c63 on 8.6.x 
    Issue #2960605 by anmolgoyal74, surbz, msankhala, arunkumark, jibran,...

Status: Fixed » Closed (fixed)

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