🇪🇸Spain @rodrigoaguilera

mradcliffe → credited rodrigoaguilera → .

The phpstan issues were solved in other commits so no need to modify the baseline anymore. Back to RTBC

Thanks for the contribution

Thank you for confirming is D10 compatible

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

Commited directly. Thanks for the contribution

I don't understand this issue. It is just for testing two issues at the same time?

Can you reroll the patch as an MR? It doesn't apply anymore to the latest 8.x-1.x branch

Looks good. Thanks for the contribution

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

I tested on the site that was giving me problems to update

Opened an MR

Usually modules changing injected services add an empty hook_post_update_NAME() to force a cache clear. Read more here:
https://www.drupal.org/node/2960601 →

To solve a single_content_sync.post_update.php file can be created with this hook.

I agree with keeping the scope of this issue small. Fixing the tests that are affected and it should be ready to go.

For me a logical followup would be to be able to customize that character instead of being hardcoded to a colon

Thanks but I'm wondering if there is any reason in particular to drop D10 compatibility on this commit
https://git.drupalcode.org/project/menu_manipulator/-/commit/b8d5335e384...

A Drupal 11 from Drupal 10 upgrade is much easier when you get all modules to be compatible with both versions and then just upgrade the core. Can you at least declare compatibility for Drupal ^10.3 ?

Additionally it is a bit confusing to have 3.1.x release on the 3.0.x branch. Maybe is time for a new branch.

If you don't want to be bothered it also fine I just want to note how other module maintainers manage their branches and releases.

I can confirm there is no other change needed for the module other than the MR2 to work on Drupal 11.

Please roll a new release with the change included

Please re-add the D11-compatibility as this module is still useful in Drupal 11

I installed the MR on a Drupal 11.1.0-rc1 install and it works as expected so I'm confident to mark it RTBC. It has many code standards changes but it doesn't hurt to include. Please declare the D11 compatibility and roll a release.

Have you contacted Rakesh?
https://www.drupal.org/user/1177822/contact →

Please read the steps for taking over a module
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

I tested manually the module on a Drupal 11.1.0-rc1 install with the changes applied and it works as expected. Upgrade status doesn't report any other problem with the module.

Please commit this and roll a new release. MR12 includes extra changes to comply with coding standards so it won't hurt to include those, so my RTBC go for that one.

I installed the 2.x branch on a Drupal 11.1.0-rc1 site and applied the compatibility patch.

I can confirm the module works as expected on Drupal 11 and that scans from the upgrade_status module don't return any other issue.

Please commit and roll a new release

ivnish is now an active maintainer so I don't think this is need anymore. Thanks

I got confused because the most updated branch was project-update-bot-only and the MR was oldest. I hid the outdated branch (opposite of what I did before).

The branch now has no deprecations running phpunit. There still one risky test but I think that is out of scope of this issue.

Glad to see one of the maintainers around :)

The MR is ready to review again

rodrigoaguilera → changed the visibility of the branch 3434446-automated-drupal-11 to hidden.

rodrigoaguilera → changed the visibility of the branch project-update-bot-only to active.

I fixed the deprecations but now there is some tests failures. Need a deeper look. I can't right now

Good catch.
Let me have a look

rodrigoaguilera → changed the visibility of the branch project-update-bot-only to hidden.

I might be wrong but it seems like bootswatch themes are compiled sith SCSS variables and not CSS variables the also include some CSS for each theme.

In my UI Suite Bootstrap subtheme I was able to apply one of the bootswatch themes with

libraries-override:
  ui_suite_bootstrap/bootstrap:
    css:
      theme:
        /libraries/bootstrap/dist/css/bootstrap.min.css: /libraries/bootswatch/dist/sketchy/bootstrap.min.css

and composer req npm-asset/bootswatch:^5

Basically ConfigFormBase now takes two arguments for the constructor

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Co...

The warning in the phpstan (next major) was already warning about it

https://git.drupalcode.org/project/user_register_notify/-/commit/efb9d23...

Please roll a release after this to be able to use the module.

rodrigoaguilera → created an issue.

Makes sense

I found this googling for a solution. A better alternative for me has been to use https://www.drupal.org/project/views_rss →

The module takes care of outputting the raw description for RSS

Moving to the ownership queue

Sure. Added tests for the formatters as well

Seems like it was an issue with not declaring the schema for the new third party setting field.
I also squashed the experiments that I did while I didn't have a proper dev enviroment.

In the last two commits I add just the test, failing in the correct place and the the simple fix.

The test only checks widgets, not formatters but since it was the same fix for both I didn't find it very useful to duplicate testing there.

Seems like trying to fail the test on purpose, changing assertNotEmpty for assertEmpty, also passes the test so I'm clueless about what is happening with that test :(

Weird. The tests are not failing where they are supposed to.

The solution to fixing the test should be in core/modules/field_ui/src/Form/EntityFormDisplayEditForm.php protected function thirdPartySettingsForm().

Adding:
$settings_form[$module] = ($settings_form[$module] ?? []) + $hook(

Filed 🐛 Support multiple implementations for hook_field_widget_third_party_settings_form Active

rodrigoaguilera → created an issue.

Apologies for the noise. Indeed multiple implementations are supported but there is special cases like hook_field_widget_third_party_settings_form that won't work the multiple implementations. Will file an issue for that.

I changed the CR to reflect that
https://www.drupal.org/node/3442349 →

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

Thanks for the link.
The project I migrated is https://git.drupalcode.org/project/change_labels/-/tree/1.x?ref_type=heads
but is already frankensteined with traits to workaround this issue that I'm talking about.

To showcase the problem I created a patch for automated_cron with similar hook_field_widget_third_party_settings_form implementations.
Only the "second" implementation end up displaying its form in the widget settings.

Maybe this is a special case since the calling module is important to later determine where to save the third party setting.

Moving one of the classes to another module (and changing the namespace) makes both form items to appear.

Can you link to the issue? Just want to investigate if there is a possibility to be added back.

I updated the CR to clarify the point about the multiple implementations
https://www.drupal.org/node/3442349 →

While migrating a module to OOP hooks I noticed that the restriction of "one implementation per module" is still there. Now is rather silent as there is no error about it. For example having two classes in the \Drupal\my_module\Hook namespace that have the same hook annotations.

I would be really convenient to have multiple implementations per module to organize related features into the same class. E.g. a module that has two field widget third party settings and each one has its hook_field_widget_third_party_settings_form with a corresponding hook_field_widget_complete_form_alter. One implementation for each of the third party setting separated in different classes.

Is this something that can be tackled in this issue or should I open a new one?

We need to add that warning somehow. First step would be the readme file

Sound like a great feature
Can you convert it to a MR?

I don't follow how you use the "target" setting as field_name. Can you explain more?

I think instead of
$entity->get($field_name)->getValue();
You can do
$entity->get($field_name)->getValue();

merged

I attempted to fix the tests by adding the tour module that became a contrib module but there is more failures.
I guess the Drupal 9 fixture is trying to upgrade directly to Drupal 11 and that creates situations like the help_topics module to be uninstalled (current test failure).

So probably adding the tour module was not a step in the right direction.
I feel the proper solution is to create a fixture that starts from Drupal 10, therefore dropping support for the upgrade path from Drupal 9.
People coming from rabbit hole v1 on Drupal 9 need to upgrade first to Drupal 10.

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

Released

Hello

Thanks for raising the issue. I do not agree on relaxing the restriction that much.
Major versions can change the structure of third party settings so those need to be checked before raising the requirements.

In this case it seems like they are compatible with drulma_companion
https://git.drupalcode.org/project/block_class/-/blob/4.0.x/config/schem...

Please review and I will merge it with a new release afterwards.

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

Added test for the deprecation so back to NR

Isn't a bit too much adding deprecation tests for a protected method?

I'm curious about how to mark them so they will be removed along the deprecated code.

I tried to look for examples but couldn't find much. I guess it would be about setting up an artificial code path with a class that extends from EntityViewsData that calls the deprecated method.

rodrigoaguilera → changed the visibility of the branch 2930736-10.1-entity-views-data-assumptions to hidden.

DeprecatedServicePropertyTrait is used for when there is a \Drupal::service() replacement call
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Dependenc...

So deprecation with no replacement I think is the appropriate thing to do here.

I rebased against 11.x and created a new MR.
Hopefully this can be merged for Drupal 11.2.0

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

rodrigoaguilera → created an issue.

I think this is valid for configuration that remains active (but not enabled) and yet invisible in the UI.
I believe the steps that I provided are still valid to reproduce this.

I don not agree that once disabled the config for the view mode should remain but if for some reason that is the intended way we can improve the UI following already existing patterns like in views where disabled display modes are still visible and they are clearly marked as disabled.

Changing the issue title to better reflect this.

I totally missed that checkbox. It doesn't fill my need because it doesn't log the body of the POST request. only this:

Request: POST http://elasticsearch:9200/general/_search?track_total_hits=true

The response is actually complete. This is probably how the original debugging module worked so the current situation is fine.

I'll open a new issue if I need to improve the logging. Thank you so much for looking into this.

What happened with this?
The current version doesn't seem to have a debug module

xjm → credited rodrigoaguilera → .

Thank you dqd for clarifying

I run manually the steps of the test on a D11 installation and it was throwing an signature compatibility error when saving nodes.
Fixed the signature in MR13

Opt-in to the next major is not the default anymore so I enabled it on explicitly
https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...

But now the current is D11 so phpunit is running with D11 and it is green, therefore I think this is ready for review.

rodrigoaguilera → changed the visibility of the branch 3434446-automated-drupal-11 to hidden.

Hello Vinay

You can open an issue for that.

Take the text from https://www.drupal.org/project/publishcontent →
and turn it in to Markdown for a readme.md file for the root of the repo. That should be one commit.
Then in successive commits you can improve that description/docs for easier review.
You should open a MR with your commits.

Once the changes have been approved by the community I can take care of using that readme.md file for updating the main page description of the module.

Please feel free to propose changes to the documentation. If the community agrees on the changes I pledge to review and merge them.

I don't dedicate much time to this module anymore but I try to pay attention to the MRs that are affecting a lot of folks.

Thanks for the reminder.

I just tagged an new version

dan2k3k4 → credited rodrigoaguilera → .

Oh, thanks for pointing that out. Then yes, they are probably a bucket for any arbitrary property.

The task of figuring out what was the original intention I think it might be difficult so I don't think is novice anymore.

Did a rebase and the pipeline is green again

I feel that array is not supposed to be extended. At least I am not aware of any module that does it.

I think the current approach in the MR is right. Leaving it for others to review

A new MR needs to be created against 11.x-dev branch.
additionally ensure all comments are addressed and the tests are all passing

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

The comments in the MR need to be looked at and figure out a way forward.

By looking at the failed tests I think there is typo "Initail"

I think the status change was missing

I guess this issue never made it into the workshop. We can work on it in Barcelona.

This issue require to write some docs into the comments of the code.

A look at the failing tests is needed

@jimiobrien Was hiding the branch an accident?

rodrigoaguilera → changed the visibility of the branch 3213995-olivero-submit-button to active.

Fix the base branch of the MR

Given the last comment I don't think is a novice issue anymore and is up to the reporter to clarify

This requires to change from the @link syntax to just the class name + method