- Issue created by @wim leers
- @wim-leers opened merge request.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
From 108 failures down to 100.
Expect this to move towards green quickly now π€
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Needs review from @longwave and @f.mazeikis.
Is IMHO RTBC-worthy and mergeable.
If need be, I could split off the bit to a separate issue + MR, but it's a single commit: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... β although that uses PHP Fibers π
- π¬π§United Kingdom f.mazeikis Brighton
f.mazeikis β made their first commit to this issueβs fork.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
@f.mazeikis FYI Iβm working on !395 as we speak β I assigned it to me 50 mins ago.
Fortunately I made the exact same changes locally, but with an extra comment explaining it π
Iβm currently investigating why:
config:search.settings
appears only in D11, not in D10X-Drupal-Cache-Max-Age
is-1
on D11, but60
on D10
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
β οΈ I cannot reproduce #8.2 locally: it passes locally with
60
!But β¦ I've been testing using commit
48b0aea1d573d9e01752f9909ec8cd3a1b7e3378
of the11.x
branch, which dates back to October 2.I already know what causes
max-age: 0
, and it's XB that ups this to 60 seconds: it's\Drupal\user\Plugin\Block\UserLoginBlock
when rendered byFormBuilder
for an authenticated user. Just pushed https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... to make that very explicit in the test coverage.something changed since then. I bet this is thanks to π Form tokens are now rendered lazily, allow forms to opt in to be cacheable Needs review β see the change record β .
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
#9's hypothesis is confirmed, although it seems somewhat accidental? π See #2578855-165: Form tokens are now rendered lazily, allow forms to opt in to be cacheable β , where I ask @catch about this.
Documented in https://git.drupalcode.org/project/experience_builder/-/merge_requests/3...
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
#8.1 is now also no longer a mystery: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3...
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Left even more "Dear reviewer, here's helpful info"-style guiding comments on the MR, prefixed with
π
.MR is now actually green and definitely ready for review.
While I still want @f.mazeikis' +1, I'm assigning to @longwave because he worked on the cacheability aspects of
ApiComponentsController
in π Add support for block derivatives Active , which this is keeping, but #3484671 is largely removing π¬ That's why I'd like this to land first, because it proves much more is possible, before we temporarily remove much of it in #3484671β¦ - π¬π§United Kingdom f.mazeikis Brighton
Reviewed and approved. It seems MR still needs approval from @tedbow or @effulgentsia.
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Approvals across the board, with one exception: technically I still need a +1 from @tedbow or @effulgentsia:
β¦ but that's for one single tiny addition to test coverage: https://git.drupalcode.org/project/experience_builder/-/merge_requests/3... β I'm confident that is not contentious.
So moving ahead and merging.
-
wim leers β
committed 2e6546b3 on 0.x
Issue #3485917 by wim leers, f.mazeikis, longwave: `...
-
wim leers β
committed 2e6546b3 on 0.x