Support from Acquia helps fund testing for Drupal Acquia logo

Comments

davmorr’s picture

Assigned: Unassigned » davmorr
davmorr’s picture

Status: Active » Needs review
FileSize
750 bytes

Changed cache_set() to cacheSet()

oenie’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/CachePluginBase.phpundefined
@@ -114,7 +114,7 @@ function cache_set_expire($type) {
+  function cacheSet($type) {

Add public access modifier in front of the function to adher to the new OOP standards.

nathangervais’s picture

Assigned: davmorr » nathangervais
Status: Needs work » Needs review
FileSize
2.2 KB

Rerolled patch to include the public access modifier and also found more occurances of this call in other files. They have been updated.

There was also a reference in a comment in core\modules\views\views.module on line 1681 that i have left unchanged as I wasnt sure this applied to this instance.

This is just a convenience wrapper around cache_set().

heddn’s picture

Status: Needs review » Needs work

Go ahead and change the comment as well.

nathangervais’s picture

Status: Needs work » Needs review
FileSize
2.69 KB

Patch rerolled with comment modified.

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/None.phpundefined
@@ -40,11 +40,11 @@ function cache_get($type) {
+   * Overrides \Drupal\views\Plugin\views\cache\CachePluginBase::cacheSet().

Let's use {@inheritdoc}

+++ b/core/modules/views/views.moduleundefined
@@ -1678,7 +1678,7 @@ function views_handler_field_custom_pre_render_move_text($form) {
- * This is just a convenience wrapper around cache_set().
+ * This is just a convenience wrapper around cacheSet().

That's out of scope because it's another cache_set meant.

nathangervais’s picture

Status: Needs work » Needs review
FileSize
2.14 KB

As near as I can tell you're asking for the entire comment block on the cacheSet function in /core/modules/views/lib/Drupal/views/Plugin/views/cache/None.php to be set to { @inheritdoc } as per https://drupal.org/node/1962592

With that assumption i've rolled this rerolled this patch to update that reference and remove the comment change in views.module.

dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/None.phpundefined
@@ -40,11 +40,9 @@ function cache_get($type) {
-   * Replace the cache set logic so it does set a cache item at all.

This contained some actual helpful information.

Let's keep this line as it explains something. Btw. it should be "... does not set a cache item at all."

nathangervais’s picture

Status: Needs work » Needs review
FileSize
2.21 KB

Rerolled Patch, readded the comment with addition of the word not.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you very much!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed a17c65c and pushed to 8.x. Thanks!

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