Comments

damiankloip’s picture

Status: Active » Needs review
StatusFileSize
new1.92 KB
dawehner’s picture

Status: Needs review » Needs work

@@ -243,27 +243,7 @@ class Block extends DisplayPluginBase {
-  protected function saveBlockCache($delta, $cache_setting) {

Shouldn't we also remove the code which is calling the function?

dawehner’s picture

Not sure whether this justifies a test :)

damiankloip’s picture

Status: Needs work » Needs review
StatusFileSize
new2.27 KB

Sorry, I thought I did that. Must have messed up my diff. It's here locally! :)

dawehner’s picture

Status: Needs review » Needs work

views.module still uses some of those variables :(

It seems to be that we need tests for that.

damiankloip’s picture

StatusFileSize
new3.13 KB
aspilicious’s picture

Issue tags: +Needs tests

I reviewed all our instances of variable_get and variable_set and we have a few views instances left we should convert to state(). Every other instance is a core instance. Yeay!

1 match in C:\xampp\htdocs\views\lib\Views\user\Plugin\views\field\Picture.php
C:\xampp\htdocs\views\tests\views_test_data\views_test_data.install
      12    return variable_get('views_test_data_schema', array());
1 match in C:\xampp\htdocs\views\tests\views_test_data\views_test_data.install
C:\xampp\htdocs\views\tests\views_test_data\views_test_data.module
      36    return variable_get('views_test_data_views_data', array());
      44    return $access && $argument1 == variable_get('test_dynamic_access_argument1', NULL) && $argument2 == variable_get('test_dynamic_access_argument2', NULL);
3 matches in C:\xampp\htdocs\views\tests\views_test_data\views_test_data.module
C:\xampp\htdocs\views\tests\views_test_data\lib\Drupal\views_test_data\Plugin\views\access\DynamicTest.php
      33      return !empty($this->options['access']) && isset($this->view->args[0]) && $this->view->args[0] == variable_get('test_dynamic_access_argument1', NULL) && isset($this->view->args[1]) && $this->view->args[1] == variable_get('test_dynamic_access_argument2', NULL);

and

2 matches in C:\xampp\htdocs\views\1817684-6.patch
C:\xampp\htdocs\views\lib\Drupal\views\Tests\ViewTestBase.php
      51      variable_set('views_test_data_schema', $this->schemaDefinition());
      52      variable_set('views_test_data_views_data', $this->viewsData());
2 matches in C:\xampp\htdocs\views\lib\Drupal\views\Tests\ViewTestBase.php
C:\xampp\htdocs\views\lib\Drupal\views\Tests\Plugin\AccessTest.php
      94      variable_set('test_dynamic_access_argument1', $argument1);
      95      variable_set('test_dynamic_access_argument2', $argument2);
xjm’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
dawehner’s picture

There is an issue for the other state() parts now: #1826244: Replace variable_get() usage in the tests with state()

dawehner’s picture

Status: Needs work » Needs review
StatusFileSize
new3.34 KB

Just a rerole for now

Status: Needs review » Needs work
Issue tags: -Needs tests, -VDC

The last submitted patch, drupal-1817684-10.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +VDC

#10: drupal-1817684-10.patch queued for re-testing.

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Woow nice.
Old legacy code--

tim.plunkett’s picture

StatusFileSize
new3 KB

Rerolled, that db_table_exists() needs to stay.

dries’s picture

Asking for a re-test.

dries’s picture

#14: vdc-1817684-14.patch queued for re-testing.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Fixed

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