Problem/Motivation

Part of #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() effort to remove drupal_static() from _update_manager_unique_identifier(), _update_manager_extract_directory() and _update_manager_cache_directory().

Proposed resolution

There's no universe where these value need resetting. Make them a function static cache.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#9 interdiff.3039074.3-9.txt1.12 KBlongwave
#9 3039074-9.patch1.44 KBlongwave
#3 3039074-3.patch2 KBclaudiu.cristea
#3 3039074-3.interdiff.txt1.52 KBclaudiu.cristea
#2 3039074-2.patch682 bytesclaudiu.cristea

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3039074-2.patch682 bytes

Patch.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Title: Remove drupal_static() from _update_manager_unique_identifier() » Remove drupal_static() from _update_manager_(unique_identifier|extract_directory|cache_directory)
Issue summary: View changes
StatusFileSize
new 3039074-3.interdiff.txt1.52 KB
new 3039074-3.patch2 KB

Extending also to _update_manager_extract_directory() and _update_manager_cache_directory().

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Needs work 
+++ b/core/modules/update/update.module
@@ -737,12 +737,12 @@ function update_storage_clear() {
- * @return
+ * @return string

While good, I think these changes to add "string" I think are unrelated.

Guess it just matters if we think a committer will kick this back to Needs Work for this.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3039074-9.patch1.44 KB
new interdiff.3039074.3-9.txt1.12 KB

Addressed #8.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

  • effulgentsia committed 49beb9d on 9.3.x 
    Issue #3039074 by claudiu.cristea, longwave, tedbow: Remove...
effulgentsia’s picture

effulgentsia commented
Status: Reviewed & tested by the community » Fixed

There's no universe where these value need resetting.

The only hypothetical use for resetting these that I can think of is if you wanted to write multiple kernel tests for UpdateManagerInstall, and didn't want them to share the same extract or cache directories (so that for example, a download from one test didn't carry over into a different test). However, we can cross that bridge if/when it comes up. E.g., by either writing functional tests instead, or refactoring these functions into a mockable service.

In the meantime, this is a good step, so pushed to 9.3.x.

Status: Fixed » Closed (fixed)

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