Cape Cod, Massachusetts
Account created on 7 March 2013, almost 12 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Adding Cookbook link.  Editing for less than 1000 characters (why????)

🇺🇸United States thejimbirch Cape Cod, Massachusetts

For the purpose of reapplying certain config actions following some events, we would have to find a way to identify and extract those from recipes when they initially get applied and keep them somewhere so that we can refer to them when needed.

I don't think it is necessary for ECA to identify and extract what config actions it wants to re-apply. I think the recipe should leave behind the ECA with the config actions it wants to apply on events. We should put that on the recipe author to create and maintain since we won't want all actions in a recipe to be re-applied.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Looks good, thank you for the contribution!

Marking as RTBC.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Shouldn't this be on one of the AI modules, and not the Paragraphs module? I don't see how this could be used unless the Paragraphs module had a dependency on the AI module which I don't think would happen.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Drupall 11 support has been added in the 3.x branch of extra_field.

composer require 'drupal/extra_field:3.0.x-dev@dev'

🇺🇸United States thejimbirch Cape Cod, Massachusetts

I think now that Drupal 11 release, the bot is no more. I have closed all of mine.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Hopefully folks on 11 are up to 11.1, but your change makes it explicit. I applied the suggestion.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

We met with @alexpott, the lead maintainer of the Recipes initiative. The long term goal of of recipes is that they are unpacked and removed from Drupal in the application process. The fact that the recipes are remaining in Drupal CMS, and this issue to forever re-apply them may lead to the recipes getting updated and havoc unleashed when reapplied.

Instead of re-applying the recipe, could there be an ECA action to apply config actions? So rather than re-applying a whole recipe, we only list the config entities and config actions we want to apply in the ECA.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Looks like a new MR has been created with less changes. Hopefully this will get us for a full Drupal 11 release. Marking as RTBC to see if @pcambra can review.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Tests pass on MR 31. Updates made by @Fabsgugu no longer give an error.

Marking as RTBC. Thanks so much!

🇺🇸United States thejimbirch Cape Cod, Massachusetts

4 Failing PHPstan tests remain.

 Line   composer/Plugin/Unpack/UnpackCollection.php                           
 ------ ---------------------------------------------------------------------- 
  36     Method Drupal\Composer\Plugin\Unpack\UnpackCollection::getIterator()  
         return type with generic class ArrayIterator does not specify its     
         types: TKey, TValue                                                   
         🪪  missingType.generics                                              
 ------ ---------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------------------------- 
  Line   core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                              
 ------ ------------------------------------------------------------------------------------------- 
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:cloneFixtureProjects\(\)            
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:createIsolatedComposerCacheDir\(\)  
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:tearDown\(\)                        
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
 ------ ------------------------------------------------------------------------------------------- 
 [ERROR] Found 4 errors               
🇺🇸United States thejimbirch Cape Cod, Massachusetts

+1 to Pam’s content suggestion.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

With the current diff from MR 34, I can't get menu links of the main menu to work.

Command:

drush dcer menu_link_content menu_name main --folder=../recipes/menus/content/

Error message:

Too many arguments to "dcer" command, expected arguments "entity_type_id" " entity_id".

If I try dce

drush dce menu_link_content main --folder=../recipes/menus/content/

I get the following error:

The "--folder" option does not exist

If I drop the folder part, I get:

Entity "menu_link_content" with ID "main" does not exist

🇺🇸United States thejimbirch Cape Cod, Massachusetts

10.4 has been released, so this can be reviewed.

🇺🇸United States thejimbirch Cape Cod, Massachusetts
🇺🇸United States thejimbirch Cape Cod, Massachusetts

MR added that adds the dependency and corrects the project's name in the composer file.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

thejimbirch changed the visibility of the branch 3503637-the-recipe-needs to hidden.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Responses in slack to that:

jurgenhaas
3 minutes ago
Good question. I'd say SEO exposes the issue, but even without SEO, a 404 response for unpublished nodes seems to be more logcial, and it won't disclose internal information. So, for me it could be a generic bug fix, not just SEO.

thejimbirch
2 minutes ago
I agree, I'd rather it in the recipe its in. That is a great ECA foundation.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Closed my issue as a duplicate of this. The solution looks promising.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Merging to dev. Thanks for the contribution!

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Merging to dev. Thanks for the contribution!

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

The code in the MR looks good and the tests make sense.

My only concern is "extra" field. Since fields are things already in Drupal, and we have a heavy lift educating folks around recipes, could we user "properties"? I don't want to bike shed or anything, just trying to lower the confusion.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Woo!

🇺🇸United States thejimbirch Cape Cod, Massachusetts
🇺🇸United States thejimbirch Cape Cod, Massachusetts

Adding them and making as Fixed. Thanks!

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Added nkroger and making as Fixed.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

I love the ECA idea. Postponing on Plugin to throw AccessDeniedException Active

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Moving to RTBC based on comments in #6, thanks!

🇺🇸United States thejimbirch Cape Cod, Massachusetts

@zoocha will Do you know their Drupal handle? I can't find one with the name "Nick Kroger" Is that everyone?

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Thanks. Adding credit and closing as fixed.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Merge request add that installs the https://www.drupal.org/project/unpublished_404 module to the drupal_cms_authentication recipe.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

thejimbirch created an issue.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Who needs to add credit here?

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Asked Catch to review in Drupal Slack since I can't assign it to him.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Strike that. Moving to Needs Review as this should have a test to validate it works. Added Needs tests tag.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Assigning to phenaproxima to review

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Closing as fixed and crediting the attendees. Please re-open if that was not the correct move.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Back to Needs Work for comments in #14

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Postponed on the forthcoming 2.1.3 release of ECA.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

There is a core issue for this.

https://www.drupal.org/project/drupal/issues/3457730 Add support for clearing the cache from the navigation menu Active

As mentioned in that issue, there is also a module:
https://www.drupal.org/project/navigation_extra_tools

🇺🇸United States thejimbirch Cape Cod, Massachusetts

This is awesome @narendrar! I made some minor suggestions.

I assume this will have to wait until the action is in a full release of ECA, and we'll have to update all the spots that are marked dev in this MR to the new release.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Follow up issue created. Lets get this one in first then. Rebased and marking as RTBC.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

MR added which removes the script and the symlink, but leaves the recipe.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Thanks for the quick response! Was literally writing my commit message, Hard codes at 1.18 until ... and came here to get the issue #.

I can confirm 1.20 no longer WSOD in 10.4.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

There is no compatibility with Drupal CMS. The compatibility is with Drupal core.

Ask the maintainer of the module to create a release that supports Drupal 11 and it will be installable on Drupal CMS in project browser.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

The decision to list only content type recipes was intentional.

Drupal CMS does not have any patches to modules as the cweagans/composer-patches plugin you added is not approved to work with core's automatic updates module, which is what Project Browser uses to manage packages. There is an unapproved config you can set to work around this until they support additional composer packages. See: https://www.drupal.org/project/automatic_updates/issues/3437845#comment-... Support oomphinc/composer-installers-extender and wikimedia/composer-merge-plugin Active

🇺🇸United States thejimbirch Cape Cod, Massachusetts

I have run into this also, WSOD with Drupal 10.4.1 and Taxonomy Entity Index 1.19.

Downgrading to Taxonomy Entity Index 1.18 removed the error.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Seems like a lot of work to add a class, and will only be available in the Drupal CMS Olivero theme.

I think how it is implemented is the Drupal way, but I would like to propose a alternate solutions.

Option 1 - Block Class

If we added the block class module, we could add a second Page Title block for the home page with the .visually-hidden and .sr-only classes, which are the most common classes in css frameworks.

Pros:
No theme hardcoding/custom code

Cons:
Ongoing support for block_class, but it really is a useful no-code tool.
Duplication of Page title block config.

Option 2 - ECA

Using the Render: add class action, we may be able to add the class to the home page block.

Pros:
No theme hardcoding/custom code
No additional modules

Cons:
Not as visible to site builders as block class.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Code looks fine. Rebased and moving to RTBC assuming the tests will turn green.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

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

🇺🇸United States thejimbirch Cape Cod, Massachusetts

This MR changes a lot of image style config entities. I think we should set precedent to document these changes with a Change record.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

Thanks for the contribution and the thorough reviews! Resolved all threads and moving to RTBC.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

@kristen pol created the follow up issue.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

thejimbirch created an issue.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

thejimbirch created an issue.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

I added an MR with Artem's notes. We need to add which config files each of the actions work on, then I think this is good to go.

🇺🇸United States thejimbirch Cape Cod, Massachusetts

thejimbirch changed the visibility of the branch 3491066-document-addnavigationblock-new to hidden.

Production build 0.71.5 2024