Hello all, it’s time for the weekly migration subsystem meeting. The meeting will take place in slack in various threads
This meeting:
➤ Is for core migrate maintainers and developers and anybody else in the community with an interest in migrations
➤ Usually happens every Thursday and alternates between 1400 and 2100 UTC.
➤ Is done on the #migration channel in Drupal Slack (see www.drupal.org/slack for information).
➤ Happens in threads, which you can follow to be notified of new replies even if you don’t comment in the thread. You may also join the meeting later and participate asynchronously!
➤ Has a public agenda anyone can add to. See the parent issue for an idea of the typical agenda.
➤*Transcript will be exported and posted* to the agenda issue. For anonymous comments, start with a :bust_in_silhouette: emoji. To take a comment or thread off the record, start with a :no_entry_sign: emoji.
The hope is that most or all of the maintainers will attend. We will try to focus on longer-term goals than in the weekly meeting.
| benjifisher |
#3281427: Update Block and Theme setting migrations to not use Bartik and Seven |
| mikelutz (he/him) |
Saw this, but haven’t looked into it. I worry there might be deeper implications during a migration than just the tests though. |
| benjifisher |
Good point. What happens when we migrate D7 -> D10 in the UI and the source site uses Bartik and/or Seven?
What currently happens if the source site is D6 using Garland? |
| mikelutz (he/him) |
Not entirely sure, but I assume the default theme is enabled via the standard profile, and the migration is not really theme dependent, except we are maybe migrating blocks into regions defined by bartik that don't exist in the new themes, in which case the tests and maybe migrations might need to be updated. |
| benjifisher |
Yes, I think the regions are what matter. |
| benjifisher |
Minor detail: we do not install Standard in the tests. We explicitly enable the themes we want to use. |
| benjifisher |
In the D6 test, I did a global replace s/bartik/olivero and got an unexpected error. Maybe I got the machine name wrong? |
| quietone |
The D6 source plugin has a region property that has a mapping that maps the Garland regions to Bartik regions. |
| mikelutz (he/him) |
And I think it needs to be updated to map to olivero regions, and the d7 migration need something similar to map from bartik to olivero. |
| benjifisher |
Maybe Seven and Claro have the same regions, so that makes less of a difference. |
| mikelutz (he/him) |
Well, plus the migrations don't really interact with the admin theme much. |
| quietone |
I worked up a patch a few days ago and mapping the regions was not easy. They are very different. |
| quietone |
Do we want to allow the ability to keep the old behavior, ie. Garland to Bartik instead of Garland to Olivero? |
| mikelutz (he/him) |
Well, that would have to mostly live with bartik in contrib. Core is going to have to support olivero by default, as best we can. |
| benjifisher |
In D9, we still have the old themes, but the new ones are the default. In D10, the old themes are moved to contrib.
See the other thread: how much do we care about consistency between D9 and D10? |
| quietone |
I just uploaded the start of a patch, #3281427: Update Block and Theme setting migrations to not use Bartik and Seven#comment-14607234 |
| mikelutz (he/him) |
Well, Olivero was default in 9.4, right? Probably should have changed the migration at the same time, but it seems reasonable to start migrating to the new theme in 9.5 on, but it would definitely need to be in 10. Since those release at the same times, that's really up to the comitter. |
| benjifisher |
#2953111: Only migrate role permissions that exist on the destination |
| benjifisher |
Progress: code review is done. |
| benjifisher |
To do:
- another round of manual testing
- update the issue summary
- update the change record
|
| benjifisher |
This issue changes behavior, with no option for keeping the old behavior. That is appropriate for D10, where you get a WSOD instead of deprecation messages if you try to save a role with invalid permissions.
What about Drupal 9.5? Should we
- make it just like D10
- provide an option to keep the old behavior
|
| quietone |
In D9 the user role migrations have this to preserve the legacy behavior.
skip_missing_permission_deprecation:
plugin: default_value
default_value: true
|
| quietone |
If someone want the new behavior they can just change the default_value to false. |
| benjifisher |
If we do not port the patch to 9.5, then that option will remain. But we should port the patch if we want to say that D10 is just like D9 except for removing the deprecated code ... and updating dependencies ... and ...
That was our stated policy going from D8 to D9. Did we decide not to make such a claim this time around? |
| quietone |
That makes sense. I will work up a patch for D9.5. |
| benjifisher |
#3285637: 'Get' Process plugin should handle multiple |
| benjifisher |
This was one of our Action Items from last week. It is now RTBC. :+1: |
| benjifisher |
Are there any lingering questions? |
| mikelutz (he/him) |
This one should be RTBC now, not sure there is anything to talk about. |
| mikelutz (he/him) |
@danflanagan8 and I were going back and forth about an additional test, that I think is unnecessary and not testing the actual issue. For now we aren’t adding it, but I’d take another opinion. |
| danflanagan8 |
I’m comfortable with the decision not to add the test. I will happily defer to the maintainers. I even called my proposed test “overtesting” before @mikelutz (he/him) used the same word. |
| benjifisher |
My understanding is that one purpose of automated tests is to illustrate how the system is supposed to work. If you agree with that, then it helps to have realistic test cases.Instead of adding a test case, what do you think of replacing the pipeline for get_from_multiple with the one suggested in Comment #12? |
| danflanagan8 |
The counterpoint to “realistic” test cases is that sometimes they add unseen and unneeded complexity, which I think was part of the argument in favor of get_from_multiple |
| mikelutz (he/him) |
My feeling was that all that adding a skip_on_empy does there is test that the pipeline from the original get (which is now injected via source: is unchanged after running it through skip_on_empty. A valid thing to test, but that belongs with the skip_on_empty tests, not in the get or multiple handling tests. |
| mikelutz (he/him) |
Kernel and unit tests in particular should be small and focused and test specific pieces. functional and JS tests should test bigger picture/realistic scenario stuff. |
Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.
Comments
Comment #2
quietone commentedDiscuss #3281427: Update Block and Theme setting migrations to not use Bartik and Seven for the block migrations and also there is
to consider.
Comment #5
quietone commentedComment #6
quietone commentedComment #7
benjifisherI reviewed the transcript and the credits. LGTM
Comment #9
benjifisherSorry, it looks as though I reverted the issue summary in my last comment. I am restoring it (as best I can) from the revision history.