Hungary
Account created on 13 June 2008, about 17 years ago
#

Merge Requests

More

Recent comments

🇭🇺Hungary mxr576 Hungary

Thanks, also for the feedback, we will try to keep up with the quality! 🙂

🇭🇺Hungary mxr576 Hungary

The scope of this conversation has widened in unexpected ways. Since many projects already have .ddev folders committed as mentioned in previous discussions, I assumed the main debate about whether this is a good practice had already been settled. However, I recognize there are valid concerns about different use cases and project types.

I'm partially surprised by the pushback. DDEV has become the de-facto development environment for Drupal projects and provides the essentials for reproducible development and QA environments. While it doesn't offer the same level of
reproducibility as Nix, it represents a significant step forward. The ddev-drupal-contrib addon is officially maintained by DDEV and provides the same flexibility and reliability for contrib development. It not only accelerates initial
project involvement but also ensures that every development environment shares identical dependency versions and tools for contribution.

I understand there are different perspectives on this, however I believe contrib modules can also benefit from standardized development environments that facilitate consistent testing and contribution workflows.

In an ideal world, Drupal.org would provide a single command to spin up DDEV development environments for contrib modules. The majority of contrib modules don't require special dependencies or third-party software for development or
testing, so the same script could cover approximately 95% of contrib projects. However, even if such a script existed - and I hope this comment catalyzes its creation - some projects would still need custom config.yaml files,
Docker files, or other overrides like version pinning committed to their repository. Therefore, the .ddev folder wouldn't disappear completely.

The idea about a single YAML file also makes sense, but I believe that would contradict DDEV's composable extensions concept, which is genuinely powerful. You can find extensions for various needs without maintaining your own custom solutions.

Having a .ddev folder also proves valuable when an extension's latest release needs tweaking or bugfixing. For example, changes from pending PRs could be committed to a module's .ddev folder today, and the
extension can be updated after the PR gets merged. This approach also enables immediate onboarding for new contributors who can run ddev start and begin contributing within minutes, rather than spending time configuring their local environment.

Perhaps we could explore a hybrid approach where the .ddev folder contains only the minimal, project-specific configuration needed, while leveraging the ddev-drupal-contrib addon for common functionality. This would reduce maintenance overhead while preserving the benefits of reproducible environments.

Ideally, in the future, boilerplate code for providing local development environments will be reduced. However, I still see significant benefits in leveraging DDEV's composability through extensions and maintaining project-specific
.ddev folders with explicit dependencies and their versions. Yes, this approach introduces minor maintenance overhead for contrib maintainers, but the benefits should outweigh the consequences.

Additionally, perhaps in the future GitLab CI could also leverage DDEV to run automated QA for modules, which would lead to truly reproducible builds across development and testing environments.

🇭🇺Hungary mxr576 Hungary

I'm glad and honored that Randy joined the conversation! I'm planning to write a comprehensive response to his comment, but I need at least one more day to put it together. If there's time to wait before closing this discussion, I would really appreciate it.

🇭🇺Hungary mxr576 Hungary

It seems to me the question got answered here. Please re-open if necessary.

🇭🇺Hungary mxr576 Hungary

I cannot approve the MR, but it looks good for me. After the patch was applied, the LLM support recipe's automated tests passed without the config schema checker bypass workaround.

🇭🇺Hungary mxr576 Hungary

Re-rolling patch to work with 11.1.x

It is better to update the existing MR to target 11.x and only upload patches for backports.

🇭🇺Hungary mxr576 Hungary

Having automated tests would also help spotting with config schema issues as it was proved in 🐛 Config schema is missing for markdownify_file_attachment.settings Active .

🇭🇺Hungary mxr576 Hungary

🐛 Config schema is missing for markdownify_file_attachment.settings Active could be fixed as part of this task too, or in scope of the dedicated ticket.

🇭🇺Hungary mxr576 Hungary

Created an MR that assumes these modules dependencies should have been there already.

🇭🇺Hungary mxr576 Hungary

Well the whole site and any content in it could be used for prompt injection to any LLM by definition, so I have removed this todo.

🇭🇺Hungary mxr576 Hungary

Well, I guess the answer is "it depends". Whoever uses that action should be awareof consequences. However without that it is complicated to set up an immediately usable demo env with recipes, where waiting for cron to index items may not be an option.

🇭🇺Hungary mxr576 Hungary

Two additional action suggestions:

  • Index items - with optional data source filtering. Also useful when default content import doesn't trigger indexing automatically - haven't checked to root cause but experienced this when I wrote a test for a recipe
  • Assign index to search backend - for recipes that bundle indexes without backends. Should filter by server type (e.g., Solr) with fallback option to prevent exception when a match not found.
🇭🇺Hungary mxr576 Hungary

I was recently looking for a way to append an item to a list property on a config entity. After some discussion on Slack and a bit of Googling, I found this issue and the proposed solution.

A few important questions came up that should be discussed and decided:

  1. Should the scope of this issue/MR be expanded to include list property manipulation on config entities? The current state of the MR explicitly excludes support for config entities.
  2. If config entities are included, how should the action be named? The current name, "simple config array", would no longer be accurate. Should we introduce a separate set of actions for simple config and config entities, or find a naming approach that can cover both?

For reference and potentially for consistency it might be worth mentioning that support for config entity updates was recently deprecated in simpleConfigUpdate: https://www.drupal.org/node/3515543 .

🇭🇺Hungary mxr576 Hungary

Add mention to Recipe Code Installer

🇭🇺Hungary mxr576 Hungary

And the answer was in front of my eyes after my new RCA... Gotta go to the corner to cry.

Thanks for saving me from another hours of debugging and sorry for the noise.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3526781-set-config-synching to hidden.

🇭🇺Hungary mxr576 Hungary

Okay, at this point, I am unsure where to catch this bug and which layers are impacted... of course setting is synching on the config installer is not a god idea since it triggers #3 by design (from \Drupal\Core\Config\ConfigInstaller::createConfiguration():

        // If we are syncing do not create configuration entities. Pluggable
        // configuration entities can have dependencies on modules that are
        // not yet enabled. This approach means that any code that expects
        // default configuration entities to exist will be unstable after the
        // module has been enabled and before the config entity has been
        // imported.
        if ($this->isSyncing()) {
          continue;
        }

setting the isSynching() on the config entity was an idea to bypass the dependency calculation in \Drupal\Core\Config\Entity\ConfigEntityBase::preSave():

    if (!$this->isSyncing()) {
      // Ensure the correct dependencies are present. If the configuration is
      // being written during a configuration synchronization then there is no
      // need to recalculate the dependencies.
      $this->calculateDependencies();
      // If the data is trusted we need to ensure that the dependencies are
      // sorted as per their schema. If the save is not trusted then the
      // configuration will be sorted by StorableConfigBase.
      if ($this->trustedData) {}

...but this changes Drupal core behavior globally, there is a way to scope it to recipes only, but still, would that be the approach? Unsure, especially even with this change other parts of the Recipe install fails when it tries to fetch the "datasource_foo" entity for different reasons (like because entity insert hooks are triggered).

🇭🇺Hungary mxr576 Hungary

New Drupal focused contents are on the horizon - one is in the review phase already - this is the reason why we thought it is also high time to get back on the Drupal Planet feed and not just promoting Drupal and our contribution on social media and other platforms.

We could also audit our old content and re-tag them with our new tag to appear in the feed, but that would not matter since the aggregator ignores older content.

🇭🇺Hungary mxr576 Hungary

and the exception originates from \Drupal\Core\Recipe\RecipeRunner::installContent()... so maybe I bumped into another issue and the proposed fix is not completely a bad idea (open for comments, please!)

🇭🇺Hungary mxr576 Hungary

Okay, so it seems the proposed change breaks the RecipeQuickStartTest with a similar error that I have reported, although the proposed fix fixes the problem that I reported.

In DiscoveryTrait.php line 53:\n
                                                                                                                                                                                                \n
  The "entity:shortcut:default" plugin does not exist. Valid plugin IDs for Drupal\Core\TypedData\TypedDataManager are: filter_format, entity_reference, entity, entity:block, entity:block_co  \n
  ntent_type, entity:block_content, entity:field_config, entity:field_storage_config, entity:filter_format, entity:path_alias, entity:shortcut_set, entity:shortcut, entity:menu, entity:actio  \n
  n, entity:user_role, entity:user, entity:view, entity:date_format, entity:entity_view_mode, entity:entity_view_display, entity:entity_form_mode, entity:entity_form_display, entity:base_fie  \n
  ld_override, field_item:link, field_item:text, field_item:text_with_summary, field_item:text_long, field_item:float, field_item:integer, field_item:decimal, field_item:string_long, field_i  \n
  tem:created, field_item:entity_reference, field_item:string, field_item:changed, field_item:map, field_item:password, field_item:timestamp, field_item:uuid, field_item:language, field_item  \n
  :uri, field_item:email, field_item:boolean, float, decimal, boolean, email, uri, list, language, map, string, duration_iso8601, timespan, timestamp, any, binary, datetime_iso8601, integer,  \n
   language_reference          
🇭🇺Hungary mxr576 Hungary

For the record, having server: null in the config also triggers this bug. So other than the inconsistency between '' and null as potential values, I think the fix in the MR is still necessary.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3163299-views-multiple-ajax-exposed-filters to hidden.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3163299-ajax-exposed-filters-9.5.x to hidden.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3163299-ajax-exposed-filters to hidden.

🇭🇺Hungary mxr576 Hungary

mxr576 created an issue.

🇭🇺Hungary mxr576 Hungary

I ran into the same issue yesterday with a Recipe that depends on BEF. BEF includes a composer.libraries.json file in its package, which you can see here:
https://git.drupalcode.org/project/better_exposed_filters/-/blob/7.0.x/c...

I agree that Wikimedia’s setup is quite complex. In our monorepo, we also treat it as a necessary evil rather than something to optimize around, so I don’t think compatibility with it should be a primary concern.

To avoid this issue in our setup, we updated our include rule from the broad composer.*.json to something more specific like composer.dev.json, which prevents BEF’s composer.libraries.json from being picked up unintentionally.

That said, I’d recommend opening a follow-up issue to improve the error reporting in the assertion logic, so it provides clearer feedback when Composer’s actual state doesn’t match the expected configuration.

🇭🇺Hungary mxr576 Hungary

The current MR diff cannot be installed with Composer patches due to changes on composer.json - that not considered anyway by Composer .

🇭🇺Hungary mxr576 Hungary

@claudiu.cristea Could you provide more context on this issue? I wonder if 🐛 View recalculated with wrong data from cache Needs review will fix this also.

🇭🇺Hungary mxr576 Hungary

@drunken monkey I would suggested merging this MR and then other Views caching related issues - if they still exits - could branch of from a new baseline.

🇭🇺Hungary mxr576 Hungary

Bubbling up cacheabilty information from the display handler may not be enough, the cacheability information from the View entity should be also incorporated - at least cache tags, that includes the View config entity's own cache tag - more details in #2887144-44: Views exposed form block options are not updated immediately when adding additional sorts, filters, etc (Caching?)

Let me also deeplink the failing test which should prove a cache invalidation issue still exists: https://git.drupalcode.org/issue/drupal-2887144/-/pipelines/500971/test_...)

🇭🇺Hungary mxr576 Hungary

The failing test confirms that even with 🐛 Extend ViewsBlockBase to merge cache metadata from display handler Active merged into Drupal core, this issue still persists. The cacheability metadata from the View config entity is still not bubbling up to the block, so the underlying problem remains unresolved.

Relying on View::getCacheTags() should be a better solution instead of a hand-crafted config cache id, since \Drupal\views\Entity\View::addCacheMetadata() ensures that before a View config is changed display plugin's cacheability information is also incorporated into the stored cache tags.

Question: What about cache contexts and cache max age bubble up from Views?

The merge request is currently failing due to the test coverage introduced in 🐛 Extend ViewsBlockBase to merge cache metadata from display handler Active , which added significant amounts of mocking. Somebody has to fix those, because I tried and failed miserably due to interconnected dependencies and protected properties... (This is one of the main reasons I try to avoid overusing mocks in my own projects - they tend to make the test suite fragile and harder to maintain. </personal_opinion>)

(There may also be a related issue in the contrib space: 🐛 Filter configuration changes does not propagate immeditelly to BEF in a block Active .)

🇭🇺Hungary mxr576 Hungary

I would consider this an outdated issue, I have tried to reproduce the issue by following the manual steps on 11.x today and I could not.

My gut says that this issue was resolved in 🐛 Extend ViewsBlockBase to merge cache metadata from display handler Active since the merge was merged fix is similar to #13.

Please re-open if you disagree.

🇭🇺Hungary mxr576 Hungary

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

🇭🇺Hungary mxr576 Hungary

Tested on my local and the fix works. Also ran the update hook.
I left a nitpick, but the current solution also works. RTBC++

🇭🇺Hungary mxr576 Hungary

I would first merge the soft limit fix and meanwhile flash out the details of this one, since this have to be configurable as well - even if it is 3 lines of CSS - it is more work the deliver.
Currently I also cannot commit to any dedicated time working on this.

🇭🇺Hungary mxr576 Hungary

. But eventually probably should have a facets related test added

Agreed, thank you!

🇭🇺Hungary mxr576 Hungary

@mably, that could also work, but I did not want to touch the core logic of filtering since fixing this specific problem was quite simple and did not require any dirty workarounds.

🇭🇺Hungary mxr576 Hungary

I would suggest creating a dedicated issue for that and please rtbc this one if it worked for you.

🇭🇺Hungary mxr576 Hungary

Fastest commit merge ever! 🙂 Thank you too

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3523983-fix-missing-config to active.

🇭🇺Hungary mxr576 Hungary

mxr576 changed the visibility of the branch 3523983-fix-missing-config to hidden.

🇭🇺Hungary mxr576 Hungary

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

🇭🇺Hungary mxr576 Hungary

I am quite confident, you do not need DDEV to reproduce this issue at all, just clone the project and try to run composer install inside the folder. It is going to fail the same way because Composer cannot resolve other dependency versions without the branch alias.

🇭🇺Hungary mxr576 Hungary

A Slack conversation started here: https://drupal.slack.com/archives/C3E9QDZ5M/p1745499507960159
Where I have also shared this link that lists all contribs hosted on Drupal Git with DDEV enabled: https://git.drupalcode.org/search?group_id=2&scope=blobs&search=filename...

Yes, most of this is auto-genarated code at the moment of DDEV and necessary DDEV addons (if any is necessary) configured for the project. The benefit of committing these files that they drastically speed up contribution to projects, because whoever has DDEV installed - the official Drupal development tool for Drupal and contribs as far as I know - could just git clone the project and run ddev start and start contributing. They do not need to follow manual steps and ensure the right set of dependencies are installed, because the committed "last known good" configuration is in the version controlling system already.

Ignoring these configs from artifacts of course is a must, because when you install a module from an artifact you most likely do not need the bundled DDEV config.

🇭🇺Hungary mxr576 Hungary

Added test coverage, incorporated the latest fix from #13.

🇭🇺Hungary mxr576 Hungary

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

🇭🇺Hungary mxr576 Hungary

It looks like the best solution would be to have #2205027: VERSION in library declarations does not work for contributed/custom modules commited in core, so we can cache our assets as best as possible.

It could be, but until that is resolved, the current solution with version: VERSION leads to cache invalidation issues at client side and CDN level when only this module is updated.

So heavy +1 on removing this constant usage for now and let Drupal to calculate the version of the asset based on the hash of the file for now.

See also the warning on https://www.drupal.org/docs/develop/theming-drupal/adding-assets-css-js-...

🇭🇺Hungary mxr576 Hungary

Checked quickly and I have only found a basic test coverage in the related sub module which might no be even using BEF in testing atm based on the export test view config :see-no-evil:

https://git.drupalcode.org/project/facets/-/tree/3.0.x/modules/facets_ex...

🇭🇺Hungary mxr576 Hungary

@smustgrave tried that, added a new checkbox multivalue widget to bef_view but could not reproduce the issue, not even with sorting enabled on items. So the real culprit is the way how facets module reorders the active item to the top.

Since Facets 3 switched to BEF as dependency, maybe it becomes inevitable having a compatibility test with it since more and more issues popping up in BEF's issue queue from there. This is how I also got here this week :-)

🇭🇺Hungary mxr576 Hungary

I do not know how to provide test coverage for this without adding Facets 3 as a dependency, did not find a way to configure a similar behavior as "The facets exposed filter is configured to order any selected options above deselected options." which is the culprit of the issue.

🇭🇺Hungary mxr576 Hungary

Just opened the upstream issue since I was always interested about having official support for the unpack feature fundamentals in Composer: https://github.com/composer/composer/issues/12401

🇭🇺Hungary mxr576 Hungary

+1 on this, I have just wanted to open a similar ticket after I have seen New Recipe Unpack composer plugin being published.

Production build 0.71.5 2024