Files: 
CommentFileSizeAuthor
#14 load-constants-2079011-14.patch20.34 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 58,858 pass(es).
[ View ]
#14 diff-8-14.txt739 bytesalexpott
#8 6-8-interdiff.txt1.15 KBalexpott
#8 load-constants-2079011-8.patch20.26 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 58,021 pass(es).
[ View ]
#6 load-constants-2079011-6.patch19.81 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 58,361 pass(es).
[ View ]
#6 load-constants-2079011-6-interdiff.txt4.4 KBBerdir
#3 load-constants-2079011-3.patch15.48 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 58,441 pass(es), 0 fail(s), and 49 exception(s).
[ View ]
#3 load-constants-2079011-3-interdiff.txt1.68 KBBerdir
#1 load-constants-2079011-1.patch14.81 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 58,136 pass(es), 1 fail(s), and 5,460 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new14.81 KB
FAILED: [[SimpleTest]]: [MySQL] 58,136 pass(es), 1 fail(s), and 5,460 exception(s).
[ View ]

There's one instance where I'm not sure what it is, see @todo.

Also weird, the storage uses this constant only internally (except field stuff), but EFQ uses it publicly, with a different meaning (load the revision vs query all the revisions). I think it would be better to create separate constants here.

Status:Needs review» Needs work

The last submitted patch, load-constants-2079011-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.68 KB
new15.48 KB
FAILED: [[SimpleTest]]: [MySQL] 58,441 pass(es), 0 fail(s), and 49 exception(s).
[ View ]

Missed two instances too aggressive with deleting...

Issue tags:+API change

Tagging.

Status:Needs review» Needs work

The last submitted patch, load-constants-2079011-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.4 KB
new19.81 KB
PASSED: [[SimpleTest]]: [MySQL] 58,361 pass(es).
[ View ]

More costant fixes.

Title:Moved field.module constants related to storage to EntityStorageControllerInterfaceMove field.module constants related to storage to EntityStorageControllerInterface

StatusFileSize
new20.26 KB
PASSED: [[SimpleTest]]: [MySQL] 58,021 pass(es).
[ View ]
new1.15 KB

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
@@ -202,6 +202,7 @@ public function query($use_groupby = FALSE) {
+      // @todo: What is this?
       $rkey = $this->definition['is revision'] ? 'FIELD_LOAD_REVISION' : 'FIELD_LOAD_CURRENT';

We can just use the constants properly here

Status:Needs review» Reviewed & tested by the community

Looks good now.

Status:Reviewed & tested by the community» Needs work
Issue tags:-API change

The last submitted patch, load-constants-2079011-8.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+API change

#8: load-constants-2079011-8.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Looks still ok :)

Thanks folks ! I'm fine with this getting in to clean obvious scope issues, but I opened #2081513: Get rid of FIELD_LOAD_* constants ? for more thoughts on those constants.

+++ b/core/modules/field/lib/Drupal/field/Plugin/views/field/Field.php
@@ -202,7 +203,7 @@ public function query($use_groupby = FALSE) {
       $fields = array();
-      $rkey = $this->definition['is revision'] ? 'FIELD_LOAD_REVISION' : 'FIELD_LOAD_CURRENT';
+      $rkey = $this->definition['is revision'] ? EntityStorageControllerInterface::FIELD_LOAD_REVISION : EntityStorageControllerInterface::FIELD_LOAD_CURRENT;
       // Go through the list and determine the actual column name from field api.

Side note: this code looks wrong / broken anyway, see #2081533: Views "group by" on "Field API" fields broken

StatusFileSize
new739 bytes
new20.34 KB
PASSED: [[SimpleTest]]: [MySQL] 58,858 pass(es).
[ View ]

Patch no longer applies :) rerolled - I'm all for getting this is and then working out if we can do without them

The interdiff was empty :) so attaching diff that shows a line that's unchanged by this patch has been changed.

Priority:Normal» Critical
Status:Reviewed & tested by the community» Active
Issue tags:+Needs change record

Committed/pushed to 8.x, thanks!

Needs a change notice.

Title:Move field.module constants related to storage to EntityStorageControllerInterfaceChange notice: Move field.module constants related to storage to EntityStorageControllerInterface
Issue tags:+Approved API change

Title:Change notice: Move field.module constants related to storage to EntityStorageControllerInterfaceMove field.module constants related to storage to EntityStorageControllerInterface
Priority:Critical» Normal
Status:Active» Fixed

Change notice created: https://drupal.org/node/2082357

Status:Fixed» Closed (fixed)

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

Issue tags:-Needs change record

Untagging. Please remove the tag when the change notification task is completed.