Comments

sudishth created an issue. See original summary.

sudishth’s picture

Status: Active » Needs review
StatusFileSize
new3.17 KB

I was add out_of_page setting Feature,
Please review

Status: Needs review » Needs work

The last submitted patch, 2: implement_out_of_page-2905255-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sudishth’s picture

Status: Needs work » Needs review
StatusFileSize
new4.8 KB

based on comment [#comment-12238432] i was updated
Please review

Status: Needs review » Needs work

The last submitted patch, 4: implement_out_of_page-2905255-4.patch, failed testing. View results

jedihe’s picture

jedihe’s picture

jedihe’s picture

Assigned: Unassigned » jedihe
StatusFileSize
new5.04 KB

Let's see if a small tweak gets the tests to pass.

jedihe’s picture

StatusFileSize
new5.32 KB
new2.13 KB

Let's now resolve most coding standards issues (only one left is to prevent a merge conflict with 2905038 #7).

jedihe’s picture

StatusFileSize
new8.38 KB

Looks like Simpletest tests got broken by an incomplete refactor of a method name.

jedihe’s picture

StatusFileSize
new2.62 KB
new11 KB

And to round it all, let's also add a simple assert for out_of_page slots; for simplicity, I just leveraged the existing Simpletest test... better to work on converting those into PHPUnit in a separate issue.

jedihe’s picture

Assigned: jedihe » Unassigned
Status: Needs work » Needs review

Ready for review; context integration is to be done at #3116001: Context integration (out-of-page slots) (once the work here is finished and merged).

jedihe’s picture

Just undoing a coding-standards fix, in order to get this patch to apply cleanly on top of #2905038: Add back token browser #7.

jedihe’s picture

Just attaching a version of #13 that can be applied on top of 2d0d5a3 + #2905298: Javascript outdated #15, should anybody want the proper async/sync switching alongside out-of-page support.

jedihe’s picture

Status: Needs review » Needs work
StatusFileSize
new5.1 KB

Re-rolling #13 for DFP 1.0, without the test. It also applies on top of DFP 1.0 + #2905298: Javascript outdated #17.

TODO:

- Port the test from #13.

jedihe’s picture

StatusFileSize
new5.06 KB

Re-rolling #15 for DFP 1.2.

It also applies on top of DFP 1.2 + #2905298-22: Javascript outdated.

No interdiff; the only conflict I had to resolve manually was that |raw was used in the twig file before DFP 1.2, but now it isn't.

TODO:

- Port test from #13.

jedihe’s picture

Tests are unstable; see #3280781: Stabilize tests for D9.3.

freelock made their first commit to this issue’s fork.

vladimiraus’s picture

Version: 8.x-1.x-dev » 2.0.x-dev

Thank for contributing!
Can we create merge request for 2.0.x branch?

freelock’s picture

Issue summary: View changes
freelock’s picture

Status: Needs work » Needs review

Setting to NR.

jedihe’s picture

StatusFileSize
new4.77 KB

I just created MR 25, since MR 22 was set to merge onto 8.x-1.x and I couldn't find a way to switch it to merge into 2.0.x.

I'm also attaching the patch from current MR 25, which applies cleanly on top of 2.0.2.

anybody’s picture

Version: 2.0.x-dev » 3.0.x-dev
Status: Needs review » Needs work

Needs a rebase