🇺🇸United States @mglaman

Added to the merge train! Thanks

I don't think this is really postponed on needing more info if the concern is BC.

Fixing the description is a workaround. But 🐛 Improve 'administer content' permission description Needs work is much more verbose. I guess we could link and close this as a dupe. But fixing the description does not fix this issue.

Merged!

LGTM!

Thanks! Merge train ahead

mglaman → created an issue.

mglaman → created an issue.

Looks good! Added to merge train

Tried to rebase via the UI and it failed on conflicts, otherwise good to go

On my side it is not enough when css aggregation is ON.

Ah! I forgot to mention we currently have CSS aggregation off due to early conflicts w/ Experience Builder (now Canvas) and I haven't re-tested with it on in a while.

Read it over. Looks good to me! I like that we're getting the Composer binary path from within it's vendor directory versus vendor/bin/composer.

chatted w/ phenaproxima and I understand the problem, now.

Sometimes after running an update, the cache becomes corrupted and has to be cleared.

This confused me. It's not _update_ as in Drupal database updates but dependency updates. A module is update which has modified it's service constructors. Drupal is still using a previous container cache for the previous version of the module. When the user goes to their Drupal site, the container constructs the services using previous arguments and we WSOD.

📌 Use a better container cache key Active solves this by using dependency version information to vary the container cache. So I think Drupal CMS can provide a Composer script which _kind of_ does the same thing. Without calling `drupal_rebuild`. When calling `drupal_rebuild` all caches get destroyed, which can be bad ahead of actually running updates.

Instead it should have a script which does clears the container cache only. This can be done via drush cc container or a PHP script like:

$autoloader = require_once __DIR__ . '/autoload.php';
$request = Request::createFromGlobals();
$kernel = new DrupalKernel('prod', $class_loader);
$kernel->setSitePath(DrupalKernel::findSitePath($request));
$kernel->invalidateContainer();

Chiming in that we've been using a patch for this since March 11th.

diff --git a/gin.libraries.yml b/gin.libraries.yml
index 45f93acd..b1212444 100644
--- a/gin.libraries.yml
+++ b/gin.libraries.yml
@@ -14,7 +14,7 @@ gin_base:
       dist/css/theme/variables.css: { minified: false }
   dependencies:
     - gin/tabs
-    - gin/dialog
+    - gin/gin_dialog
     - gin/status

 # Legacy
@@ -165,7 +165,6 @@ gin_dialog:
       dist/css/theme/dialog.css: { minified: false }
   dependencies:
     - gin/dialog
-    - gin/gin_base
     - gin/gin_accent

 gin_description_toggle:

Although I haven't exactly tested without the patch in a while.

++ great idea

I think 📌 Support redirects to non-content entity pages for example a view Active may have fixed this.

Looks like I missed this issue when merging #3180737: Add path_alias module as dependency. → :/. I'll make sure to save credit since it was my mistake to not credit before and merge issues.

Thanks!

Rolling on the merge train. Thanks!

Sorry, didn't catch the fail on type hinted constants breaking previous major

LGTM. Basically this allows redirects to always return their value and not a 404, which is how external redirects work anyway

Now that 📌 Only register decoupled_router.redirect_path_translator.subscriber if the Redirect module is installed Active is in, can we rebase off of that and tidy that up here?

Thanks!

mglaman → changed the visibility of the branch 3111456-resolve-langcode-issue to active.

given this actually works on the old redirect repository as we're only passing in an extra parameter.

oh, I didn't test so I thought it was a breaking change. so it's basically backwards compatible? I see, `findMatchingRedirect` added cacheability. And for folks who don't upgrade we lose the cacheability because we no longer do this, correct?

    // If there is a response object, add the cacheability metadata necessary
    // for the traced URLs.
    array_walk($traced_urls, function ($traced_url) use ($response) {
      $response->addCacheableDependency($traced_url);
    });

LGTM if OK changing -dev requirement to ai -dev

That's because the namespace is `january_theme` but the code is `january`. See https://git.drupalcode.org/project/january_theme

We assume the project namespace matches the code in the theme/module.

Looks good to me!

10 | WARNING | Interface names should always have the suffix "Interface"
    |         | (Drupal.Classes.InterfaceName.InterfaceSuffix)

Didn't read phpcs fail, turns out coding standards dictated the answer.

I'll set this to 1.2.x, since I can't see any reasons that this would be a breaking change?

Yeah nothing should break. The interface isn't adding methods at all.

I'll try to do some more debugging vs just reading calls from the Blackfire graph and report back. We also have several Metatag contrib running.

mglaman → created an issue.

Left a review. I don't love the approach as sites could easily be in a broken state since we can't bump redirect. There's no peerDependency type declaration we could provide

mglaman → made their first commit to this issue’s fork.

Thanks!

Let's do it!

So 🌱 Guidelines for semantic versioning and Drupal core support Active led to https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... → and technically we should do a minor release if dropping 9.x.

🤷‍♂️ although does the minor version mean anything if we merge onto 2.x and just tag 2.1.0? It's not like we're supporting 2.0.x in any true fashion and backporting fixes.

D9 is long gone that I'm +1 to doing this in a patch release.

We're adding extra libraries to the editor using KernelEvents::RESPONSE => ['onRespond', 50] for $route_name === 'entity.xb_page.edit_form' || $route_name === 'experience_builder.experience_builder' and:

        $response->addAttachments([
          'library' => [
            'module/our-library',
          ],
        ]);

mglaman → created an issue.

I've had better luck with Playwright by using their sharding over multiple jobs. We have multiple jobs and pass `--shard=1/3` or `--shard=2/3`, etc to each. Then Playwright knows how to chunk the tests.

We've also used some tagging for specific tests as well, like `--grep=@multilingual`. But then other jobs need `--grep-invert=@multilingual`

Thank you for the work! Left a review

@narendrar ah yeah, I see. I was getting that exception. But the problem is that there are so many AI module exceptions without a way of knowing it is an AI module one without tracking each. I opened ✨ Provide an exception interface all exceptions implement Active so it'd be easier to catch this.

I'm guessing we should just always return the exception message regardless. We're not stable yet. ✨ Dispatch AiExceptionEvent when a provider throws an exception Active is a feature request I made that allows third parties to customize exception messages handled by integrations like XB AI.

Thanks, all!

LGTM!

Merged, thanks!

smustgrave → credited mglaman → .

This would be great for OpenAPI documentation and other JSON schema documentation generation of entities beyond Views.

which is not very helpful in this scenario.

How isn't it very helpful?

mglaman → created an issue.

mglaman → created an issue.

mglaman → created an issue.

@mglaman: that screenshot in the issue summary, which route is that for, and for how big of a component tree is it?

It was the GET on experience_builder.api.layout.get. The tree wasn't huge, maybe 12 components with various nested slots.

https://github.com/simplytestme/website/pull/509

It isn't too expensive, but this does help avoid a call to the cache backend from the access policy variation cache. Thanks for the merge!

Fixed by https://github.com/simplytestme/website/pull/503

https://github.com/simplytestme/website/pull/502 fixed commerce

https://github.com/simplytestme/website/pull/503 will fix Drupal CMS

I don't think the default content is being imported properly, but they are building.

I'll close this and continue investigating.

🐛 Simplytest.me fail on umami Active fixed Umami and others not having and installed DB.

Starshot needs to move to Drupal CMS

Commerce needs stability flag fix

The base preview has `echo "memory_limit = 512M" >> /usr/local/etc/php/conf.d/my-php.ini`. which looks like it's being used.

This will fix it: https://github.com/simplytestme/website/pull/501, tested locally

This is merged, should be available for new previews in ~10 minutes!

Okay, now it failed on

Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 5242880 bytes) in /var/lib/tugboat/stm/vendor/symfony/http-foundation/Response.php on line 15
Command Failed (Tugboat Error 1064): Exit code: 255; Command: cd ${DOCROOT} && ../vendor/bin/drush si demo_umami --db-url=mysql://tugboat:tugboat@mysql:3306/tugboat --account-name=admin --account-pass=admin -y

Which is why we passed php -d memory_limit to Drush

We can, I'll add it as part of the builds. Sorry it took so long, but I have a PR up and closing as a upde for 🐛 Cannot manually test Commerce due to bcmath extension Needs review

https://github.com/simplytestme/website/pull/499

https://github.com/simplytestme/website/pull/498 should fix it and other related issues

Ah this is caused by invoking Drush with `php`

I wonder if this was a hiccup on Tugboat end. Because email capture is part of the platform. Seeing Tugboat Error 1064 makes me think that is the case. See https://docs.tugboatqa.com/building-a-preview/preview-deep-dive/inside-a...

It seems like things are OK now

This specifically needs to be handled in \Drupal\simplytest_projects\CoreVersionManager::updateData

Guzzle has \GuzzleHttp\RetryMiddleware has via \GuzzleHttp\Middleware::retry just need to register as http_client_middleware

Merged, should be live in 10!

All right! https://github.com/simplytestme/website/pull/496 should install theme module dependencies when its the main project or an additional project once merged

mglaman → created an issue.

mglaman → created an issue.

Came up with an idea to get install theme modules first: https://github.com/simplytestme/website/pull/496

Will test manually on Sunday

Okay, now I underatand. `../vendor/bin/drush theme:enable` doesn't enable dependencies. The theme depends on the modules being installed.

Updating dependencies
Lock file operations: 4 installs, 0 updates, 0 removals
  - Locking drupal/bootstrap (3.38.0)
  - Locking drupal/jquery_ui (1.7.0)
  - Locking drupal/jquery_ui_draggable (2.1.0)
  - Locking drupal/jquery_ui_resizable (2.1.0)

they are definitely present.

Okay this is actually part of a lingering quirk I never solved: where a module has multiple major versions supported by a major version of Drupal core. Fix incoming: https://github.com/simplytestme/website/pull/494

That's just for 5.x not showing in latest, though. Need to see why 3.x failed to build.

Could this have anything to do with the fact that 3.x releases are tagged with an 8.x- prefix and 5.x releases are not?

That's my thought. But I'm pretty sure we support semver and legacy versioning. Just for some reason. It's been 4 years since I touched this specific code (!!) so my brain is rusty

nicxvan → credited mglaman → .

ProjectVersionManager::organizeAndSortReleases needs to be debugged

https://github.com/simplytestme/website/blob/main/web/modules/custom/sim...

Weird, the 3.x branch has those dependencies in composer.json. Also, weird, 5.0.2 isn't at the top of the list. It should be at the top of "Latest" not in other releases.

mglaman → created an issue.

I think the old version stored each launch and log in the database, and that's what allowed this feature. However, clicking back _should_ have gone to '/tugboat/progress/{instance_id}/{job_id}' to show the job log as long as the preview exists in Tugboat.

I'm workin on other maintenance and will see how we can bring this back.