Ronseal issue.

Files: 
CommentFileSizeAuthor
#47 1947720-47.patch168.82 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 55,365 pass(es).
[ View ]
#41 1947720-41.patch183.47 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1947720-41.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#39 1947720-39.patch187.07 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#37 1947720-37.patch187.06 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]
#33 1947720-33.patch185.87 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1947720-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#33 interdiff-1947720-33.txt303 bytesdamiankloip
#29 1947720-29.patch185.97 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,844 pass(es), 1 fail(s), and 1 exception(s).
[ View ]
#26 1947720-26.patch180.12 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 52,022 pass(es), 122 fail(s), and 3 exception(s).
[ View ]
#24 1947720-24.patch180.19 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 54,308 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#24 interdiff-1947720-24.txt579 bytesdamiankloip
#22 1947720-22.patch179.83 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 54,272 pass(es), 17 fail(s), and 2 exception(s).
[ View ]
#16 1947720-16.patch180.4 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 53,992 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#16 interdiff.txt800 bytesdamiankloip
#12 1947720-12.patch180.11 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,838 pass(es), 163 fail(s), and 1 exception(s).
[ View ]
#12 interdiff.txt2.34 KBdamiankloip
#8 1947720-8.patch182.45 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,774 pass(es), 163 fail(s), and 1 exception(s).
[ View ]
#8 interdiff.txt84.81 KBdamiankloip
#5 1947720.patch197.54 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 46,843 pass(es), 162 fail(s), and 0 exception(s).
[ View ]

Comments

Berdir’s picture

damiankloip’s picture

Assigned:Unassigned» damiankloip

Rolling now

damiankloip’s picture

Assigned:damiankloip» Unassigned
Status:Active» Postponed

Change of plan, let's postpone on the refrenced issue in #1

damiankloip’s picture

Title:Add Drupal::state() to replace state()» Use Drupal::state() to replace state()
damiankloip’s picture

StatusFileSize
new197.54 KB
FAILED: [[SimpleTest]]: [MySQL] 46,843 pass(es), 162 fail(s), and 0 exception(s).
[ View ]
Berdir’s picture

+++ b/core/includes/bootstrap.incundefined
@@ -907,7 +907,7 @@ function drupal_get_filename($type, $name, $filename = NULL) {
-        $file_list = state()->get('system.' . $type . '.files');
+        $file_list = \Drupal::state()->get('system.' . $type . '.files');

It's not necessary to use \ in non-namespaced code.

+++ b/core/includes/update.incundefined
@@ -1483,7 +1483,7 @@ function update_config_manifest_add($config_prefix, array $ids) {
  * Provides a generalized method to migrate variables from 7.x to 8.x's
- * state() system.
+ * \Drupal::state() system.

+++ b/core/modules/system/system.installundefined
@@ -734,7 +734,7 @@ function system_schema() {
-    'description' => 'Generic key-value storage table. See state() for an example.',
+    'description' => 'Generic key-value storage table. See \Drupal::state() for an example.',

I don't think this should talk about the function/method at all, it should IMHO just be "state system". Neither the function nor the method is a system, whatever's behind is.

+++ b/core/includes/utility.incundefined
@@ -49,7 +49,7 @@ function drupal_var_export($var, $prefix = '') {
-    // magic method __set_state() leaving the export broken. This
+    // magic method __set_\Drupal::state() leaving the export broken. This

+++ b/core/misc/ckeditor/ckeditor.jsundefined
@@ -751,7 +751,7 @@ j.style.cssText||"";if(!e)j.style.cssText="position: static; overflow: visible";
-setTimeout(function(){m.setStyle("display","block")},0)}b.fire("resize")}this.toggleState();if(q=this.uiItems[0]){p=this.state==CKEDITOR.TRISTATE_OFF?i.maximize.maximize:i.maximize.minimize;q=CKEDITOR.document.getById(q._.id);q.getChild(1).setHtml(p);q.setAttribute("title",p);q.setAttribute("href",'javascript:void("'+p+'");')}if(b.mode=="wysiwyg")if(k){CKEDITOR.env.gecko&&h(b);b.getSelection().selectRanges(k);(r=b.getSelection().getStartElement())&&r.scrollIntoView(true)}else j.$.scrollTo(l.x,l.y);
+setTimeout(function(){m.setStyle("display","block")},0)}b.fire("resize")}this.toggle\Drupal::state();if(q=this.uiItems[0]){p=this.state==CKEDITOR.TRISTATE_OFF?i.maximize.maximize:i.maximize.minimize;q=CKEDITOR.document.getById(q._.id);q.getChild(1).setHtml(p);q.setAttribute("title",p);q.setAttribute("href",'javascript:void("'+p+'");')}if(b.mode=="wysiwyg")if(k){CKEDITOR.env.gecko&&h(b);b.getSelection().selectRanges(k);(r=b.getSelection().getStartElement())&&r.scrollIntoView(true)}else j.$.scrollTo(l.x,l.y);

+++ b/core/misc/jquery.form.jsundefined
@@ -433,7 +433,7 @@
-        function checkState() {
+        function check\Drupal::state() {

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Form/Ajax/Display.phpundefined
@@ -29,7 +29,7 @@ public function getFormKey() {
-   * Overrides \Drupal\views_ui\Form\Ajax\ViewsFormBase::getFormState().
+   * Overrides \Drupal\views_ui\Form\Ajax\ViewsFormBase::getForm\Drupal::state().

+++ b/core/modules/views/views_ui/lib/Drupal/views_ui/Form/Ajax/ViewsFormBase.phpundefined
@@ -57,7 +57,7 @@ protected function setType($type) {
-   * Implements \Drupal\views_ui\Form\Ajax\ViewsFormInterface::getFormState().
+   * Implements \Drupal\views_ui\Form\Ajax\ViewsFormInterface::getForm\Drupal::state().

;)

xjm’s picture

#6 is exactly why we should split up the config() patch. :)

damiankloip’s picture

StatusFileSize
new84.81 KB
new182.45 KB
FAILED: [[SimpleTest]]: [MySQL] 46,774 pass(es), 163 fail(s), and 1 exception(s).
[ View ]

Sorry, that was silly, just use a regex to do the replacement! :)

xjm, not sure what you mean, but yes, splitting things up is good. one issue per Drupal helper function.

Berdir’s picture

@damiankloip: That was in reference to our discussion in #1957142: Replace config() with Drupal::config(). We agree that it should be at least an issue per function but @xjm is suggesting to split it up even more.

And, I'm not convinced by that by #6 :) Most of the things that I pointed out would have been syntax errors and changes in .js files are also obviously unrelated. Anyway, let's get the main issue in and then we'll see how easy this one will go on for example :)

xjm’s picture

lol @ 84K interdiff

I'll stop abusing this issue and discuss over in #1957142: Replace config() with Drupal::config()...

xjm’s picture

+++ b/core/modules/field/field.form.incundefined
@@ -212,10 +212,10 @@ function field_form_get_state($parents, $field_name, $langcode, &$form_state) {
- *   The array of data to store. See field_form_get_state() for the structure
+ *   The array of data to store. See field_form_get_Drupal::state() for the structure
...
- * @see field_form_get_state()
+ * @see field_form_get_Drupal::state()

Well, this patch should either be broken up, or use a better regex. ;)

damiankloip’s picture

StatusFileSize
new2.34 KB
new180.11 KB
FAILED: [[SimpleTest]]: [MySQL] 46,838 pass(es), 163 fail(s), and 1 exception(s).
[ View ]

I don't think you can treat my lazy patch rolling as a good reason for splitting this into lots of issues. I will move that dicussion over there though :)

damiankloip’s picture

Status:Postponed» Needs review

Status:Needs review» Needs work

The last submitted patch, 1947720-12.patch, failed testing.

Berdir’s picture

Ah, looks like DUBT is missing the definition for the state service and it worked so far because state() still accessed the keyvalue service. My state cache issue has the fix for this too.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new800 bytes
new180.4 KB
FAILED: [[SimpleTest]]: [MySQL] 53,992 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Yep, you're right :) This should fix it...

Status:Needs review» Needs work

The last submitted patch, 1947720-16.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review

#16: 1947720-16.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 1947720-16.patch, failed testing.

damiankloip’s picture

Not sure what is going on here, locally it is fine.

Berdir’s picture

I can reproduce this with run-tests.sh, haven't looked into it.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new179.83 KB
FAILED: [[SimpleTest]]: [MySQL] 54,272 pass(es), 17 fail(s), and 2 exception(s).
[ View ]

Just a reroll.

Status:Needs review» Needs work

The last submitted patch, 1947720-22.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new579 bytes
new180.19 KB
FAILED: [[SimpleTest]]: [MySQL] 54,308 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

There was a state() call left from the field entity patch that went in.

I think we still have the AddFeedTest issue though, not sure what is going on with that still...

Status:Needs review» Needs work

The last submitted patch, 1947720-24.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new180.12 KB
FAILED: [[SimpleTest]]: [MySQL] 52,022 pass(es), 122 fail(s), and 3 exception(s).
[ View ]

Let's reroll, and see if it's still failing.

Status:Needs review» Needs work

The last submitted patch, 1947720-26.patch, failed testing.

damiankloip’s picture

I'll reroll, that was just a git reroll. There are a few usages of state() being used since this patch was last properly rolled.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new185.97 KB
FAILED: [[SimpleTest]]: [MySQL] 55,844 pass(es), 1 fail(s), and 1 exception(s).
[ View ]

Rerolled, still not sure wtf is going on with the drupal_add_feed tests. Something on webTestBase?

Status:Needs review» Needs work

The last submitted patch, 1947720-29.patch, failed testing.

Berdir’s picture

+++ b/core/modules/system/system.installundefined
@@ -1,4 +1,4 @@
-<?php
+#<?php

This is why :)

Berdir’s picture

+++ b/core/modules/system/system.installundefined
@@ -1,4 +1,4 @@
-<?php
+#<?php

This is why :)

damiankloip’s picture

StatusFileSize
new303 bytes
new185.87 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1947720-33.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Haha, what a great interdiff.

Berdir’s picture

Status:Needs work» Needs review

.

Berdir’s picture

... double post.

Status:Needs review» Needs work

The last submitted patch, 1947720-33.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new187.06 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Rerolled

Status:Needs review» Needs work

The last submitted patch, 1947720-37.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new187.07 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed during invocation of run-tests.sh.
[ View ]

Please!

Status:Needs review» Needs work

The last submitted patch, 1947720-39.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new183.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1947720-41.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Some leftover views changes crept in. This SHOULD be good.

Status:Needs review» Needs work

The last submitted patch, 1947720-41.patch, failed testing.

Berdir’s picture

Status:Needs work» Needs review

#41: 1947720-41.patch queued for re-testing.

tstoeckler’s picture

Status:Needs review» Reviewed & tested by the community

Yup. Let's get this in. Almost 200K aren't that cool to review...

catch’s picture

#41: 1947720-41.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1947720-41.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new168.82 KB
PASSED: [[SimpleTest]]: [MySQL] 55,365 pass(es).
[ View ]

Rerolled. Can you we this is asap please - It's a pain to reroll!

catch’s picture

Status:Needs review» Reviewed & tested by the community

Putting this back into the RTBC queue so it's ready to go once the bot comes back green.

damiankloip’s picture

Great- thank you!

catch’s picture

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Status:Fixed» Closed (fixed)

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