🇩🇪Germany @Harlor

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

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

harlor → created an issue.

Ah OK the tests did pass at a rerun now.

Back to needs review.

I'm not sure if the test failures have anything todo with the changes in this issue. I hope someone finds time to find this out.

I created a new MR based on 11.x with the diff from MR 3106.

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

harlor → created an issue.

harlor → created an issue.

Maybe we should move the menu migration to an optional sub module. There might be potential use cases where someone doesn't need a menu migration or menu_item_extras...

But that can wait... The code looks good

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

Since we only add tests I guess this won't break anything...

harlor → created an issue.

harlor → changed the visibility of the branch 3511208-add-dependency-on to hidden.

harlor → changed the visibility of the branch 3511208-add-dependency-on to active.

harlor → changed the visibility of the branch 3511208-add-dependency-on to hidden.

harlor → created an issue.

harlor → created an issue.

Good points!

harlor → created an issue.

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

harlor → created an issue.

+1 Any updates on this?

harlor → created an issue.

I've tested https://git.drupalcode.org/project/drupal/-/merge_requests/4060 and it worked very well!

harlor → created an issue.

harlor → created an issue.

I fixed these issues: 📌 Fix issues reported in security advisory coverage application Needs work

harlor → created an issue.

harlor → created an issue.

Thank you so much!

Unfortunately I still got some "must be compatible with..." Fatals

Agree.

I'd like to address some issues before having a stable release:

1. The module has to have automatic tests 📌 Add test for example module Active
2. Form decorator plugins should definitely support Attributes. And maybe we should even drop or at least deprecate the support for annotations. 📌 Add support for Attribute-based plugins Active
3. (optional) It would be nice to have a decorator priority ✨ Decorator priority Active

harlor → created an issue.

Yayy tests are running :D

Thank you!

Can we separate the tests from the cleanups? Because I'd merge the tests right away.

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

harlor → created an issue.

harlor → created an issue.

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

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

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

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

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

Sorry accidentally changed status...

Great! - @graber are you planning to release a 4.3.2 for this?

harlor → created an issue.

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

harlor → created an issue.

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

Code looks good.

I think we should add a custom error message though - if one uses a invalid slot-id currently the message reads:

The machine-readable name must contain only lowercase letters, numbers, and underscores.

Which is not correct because we allow "-" too

harlor → created an issue.

harlor → created an issue.

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

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

Yes - I think I wanted to avoid merge conflicts :D