Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

Status: Active » Needs review
FileSize
17.49 KB
andypost’s picture

Issue tags: +Field API
FileSize
17.5 KB
815 bytes

Missed one place

andypost’s picture

Issue tags: +Novice

tagged for office hours review

Status: Needs review » Needs work

The last submitted patch, 1953410-cud-1981316-2.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
17.7 KB
1.07 KB

Fix broken test

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

#5: 1953410-cud-1981316-5.patch queued for re-testing.

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

The last submitted patch, 1953410-cud-1981316-5.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
FileSize
16.35 KB

Rerolled

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Looking good

aspilicious’s picture

Status: Reviewed & tested by the community » Needs review

back to nr for now, other issue has some interesting comments

aspilicious’s picture

FileSize
21.72 KB
28.14 KB

Some changes to make use of the objects in stead of the bc layer.

Status: Needs review » Needs work

The last submitted patch, 1981316-13.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
28.1 KB

Hand edited patches FTW

Status: Needs review » Needs work

The last submitted patch, 1981316-15.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
28.1 KB

Hmm moe like a reroll. I wonder why there are so many unrelated tests failing...

Status: Needs review » Needs work

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

aspilicious’s picture

Status: Needs work » Needs review
FileSize
27.31 KB

Hopefully green. I had to manually reroll large pieces, hopefully it worked...

Status: Needs review » Needs work

The last submitted patch, 1981316-19.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
27.27 KB

Status: Needs review » Needs work

The last submitted patch, 1981316-21.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review
FileSize
27.3 KB

:(

Status: Needs review » Needs work

The last submitted patch, 1981316-23.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
28.66 KB
13.5 KB

Let's see what the testbot thinks. Also attaching an interdiff, not sure what chunks of this patch have been actuallly included in the sandbox.

swentel’s picture

This is looking great too, just one remark, after that it's RTBC.

- edit - RTBC if it's green of course ;)

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageFieldDefaultImagesTest.phpundefined
@@ -51,30 +51,28 @@ function testDefaultImages() {
-      'label' => $instance['label'],
-      'required' => $instance['required'],
+      'label' => $instance->id(),

Let's use $instance->label instead of ->id().

Status: Needs review » Needs work

The last submitted patch, 1981316-25.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
28.88 KB
14.36 KB
2.95 KB

Moved the field name declaration upper for the link tests and fixed @swentel comment on #26

aspilicious’s picture

Looking great thnx for finishing this!

Status: Needs review » Needs work

The last submitted patch, 1981316-28.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
28.89 KB

Phew, core is moving fast, here's a confict fix on #28, no further changes.

andypost’s picture

RTBC. Only small nitpick, this should be commited to sandbox

+++ b/core/modules/image/lib/Drupal/image/Tests/ImageFieldDefaultImagesTest.phpundefined
@@ -51,30 +51,28 @@ function testDefaultImages() {
+      'label' => $instance->label,

let's use label() method to make it easy convert ot NG

swentel’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
706 bytes
28.89 KB

Indeed. Great work all!

andypost’s picture

+1 rtbc, @swentel please merge the patch to sandbox

alexpott’s picture

Issue tags: -Novice, -Field API

#33: 1981316-33.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

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

andypost’s picture

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

#33: 1981316-33.patch queued for re-testing.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

back to rtbc

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed a6a4adc and pushed to 8.x. Thanks!

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