Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

Status: Active » Needs review
FileSize
59.2 KB

also fixed doc-block comment

andypost’s picture

Issue tags: +Novice

tagged to office hours

swentel’s picture

Status: Needs review » Closed (won't fix)

We're going todo this in one patch.

swentel’s picture

Status: Closed (won't fix) » Needs review

So we agreed to this in chunks anyway.

swentel’s picture

Issue tags: -Novice, -Field API

#1: 1953410-cud-1981314-1.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Novice, +Field API

The last submitted patch, 1953410-cud-1981314-1.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
FileSize
67.23 KB

rerolled - not sure if the diff was taken right as it's so big, but I assume it's ok.

Status: Needs review » Needs work

The last submitted patch, 1981314-7.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review

You removed to much ;)

Hopefully this one will apply and come back green

aspilicious’s picture

FileSize
68.18 KB

Here is the patch. I don't think we should clean this one up as the others. Will be an unneeded bikeshed issue.

I hope for less than 5 fails :p

Status: Needs review » Needs work

The last submitted patch, 1981314-10.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
28.56 KB

ANother try

Status: Needs review » Needs work
Issue tags: -Novice, -Field API

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

aspilicious’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +Field API

#12: 1981314-12.patch queued for re-testing.

pcambra’s picture

Assigned: Unassigned » pcambra

Working on this, I'll post a patch in a few mins.

pcambra’s picture

FileSize
104.17 KB
49.86 KB

#12 is a patch from another issue :(

Rerrolling #10 and fixing a number of docs, scopes and typos.

Also, are we keeping the field crud functions in field.crud.inc for compatibility purposes or shall we delete those as well?

pcambra’s picture

FileSize
112 KB
7.83 KB

After discussing with swentel on IRC, we're removing the field crud functions in this patch.

Expect this one to fail horribly, as all the dependent ones are going to still call the crud functions.

Status: Needs review » Needs work
Issue tags: -Novice, -Field API

The last submitted patch, 1981314-17.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +Field API

#17: 1981314-17.patch queued for re-testing.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Let's finish it!

aspilicious’s picture

#17: 1981314-17.patch queued for re-testing.

effulgentsia’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/field/lib/Drupal/field/Tests/BulkDeleteTest.php
@@ -104,29 +143,38 @@ function setUp() {
+        $instance = entity_create('field_instance', array(
+          'field_name' => $field->id(),
           'entity_type' => $this->entity_type,
           'bundle' => $bundle,
-        );
-        $this->instances[] = field_create_instance($instance);
+        ))->save();

save() does not return the entity. It actually returns a constant. How is it this isn't triggering test errors?

pcambra’s picture

Status: Needs work » Needs review
FileSize
112.04 KB
1.22 KB

How is it this isn't triggering test errors?

Because apparently $this->instances is not invoked from anywhere.

Re-roll removing the instances array as it's unused.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed again and I see no nitpicks

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Woot! Committed to 8.x. Thanks.

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