Problem/Motivation
Drupal 10 is coming! Let's make this compatible. Also, the test coverage in Layout Builder Restrictions includes an integration test with this module, so for tests to pass on D10, this module needs to have a D10 compatible version (see #3257889: Drupal 10 compatibility.
Audit procedure
1. palantirnet/drupal-rector's static analysis suggested simplifying some of the namespace declarations in classes. I suggest we adopt those suggestions.
2. mglaman/drupal-check reported only what I believe to be false positives, along with a valid deprecation notice (#3091432: SectionStorageTrait is deprecated in favor of SectionListTrait)
3. The upgrade_status module also reported the deprecation of SectionStorageTrait, and called out the info.yml D10 compatibility requirement.
Proposed resolution
1. Adopt the automated fixes from drupal-rector.
2. Replace the single instance of SectionStorageTrait with SectionListTrait. Since SectionListTrait was introduced in Drupal 9.3, this module will have a new minimum core requirement of 9.3.
3. Update the core version requirement to ^9.3 || ^10
Testing steps to use
1. Install latest version of D10 development snapshot
2. Enable the version of this module in the merge request
3. Go to /admin/structure/mini_layouts
4. Create a mini layout instance (of type "Layout Section") and add a section and a block to its layout.
5. Go to a Layout Builder-enabled entity and add the block created above to the layout & save.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | 3296076-d10-compatibility_12.patch | 1.11 KB | mark_fullmer |
Issue fork mini_layouts-3296076
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3296076-2
changes, plain diff MR !3
- 3296076-drupal-10-compatibility
changes, plain diff MR !2 /
changes, plain diff MR !1
Comments
Comment #3
mark_fullmerComment #4
kristen polThanks for the issue and MR.
This module needs functional testing on Drupal 10. Adding general instructions:
https://www.drupal.org/community-initiatives/contribution-events-initiat...
Comment #5
Aamir M commentedComment #6
Aamir M commentedThe MR ! 1 provided is applied successfully and working for me.
The module is compatible with Drupal 10 and also working fine.
Screenshots are attached for reference.
Comment #7
mark_fullmerI can affirm the functional nature of this in D10, as above. Marking as RTBC!
Comment #8
phenaproximaI just reviewed the merge request and a lot of it appears to be out-of-scope changes that simply remove the namespacing from parameter doc comments. Although that's not harmful, as far as I know, I would humbly suggest we revert those changes (especially since they violate Drupal's coding standards) and only change what's needed to get this D10 ready.
Comment #9
mark_fullmerI appreciate any and all humble suggestions from phenaproxima :)
I'll revert the out-of-scope coding syntax changes in the MR later today, and will double-check what syntax checking I was using that apparently was divergent from Drupal standards, re namespacing in parameter doc comments.
Comment #10
phenaproximaIt looks like the kind of thing that might have been automatically done by an IDE, to be honest.
Comment #11
mark_fullmerOkay, the unrelated code syntax changes have been backed out. The new MR retains only the D10 compatibility elements (which are few!)
Since this module currently has no automated tests, I've added manual testing steps to the issue description, copied below for reference:
Testing steps
1. Install latest version of D10 development snapshot
2. Enable the version of this module in the merge request
3. Go to
/admin/structure/mini_layouts4. Create a mini layout instance (of type "Layout Section") and add a section and a block to its layout.
5. Go to a Layout Builder-enabled entity and add the block created above to the layout & save.
Comment #12
mark_fullmerI'm seeing some odd pipeline behavior on Gitlab (or maybe I just don't know how to use it?!), so I'm attaching a patch as an alternative.
Comment #13
phenaproximaComment #17
phenaproximaOne thing maybe worth mentioning here: if we can't get Mini Layouts D10-ready, we could also remove the dev dependency from this module, and any tests which require mini_layouts could be (automatically?) skipped by adding a
@requires module mini_layoutsannotation to the relevant test classes.Comment #18
mark_fullmerI like the above suggestion regarding Layout Builder Restrictions' path to D10-compatibility (which has an integration test with mini_layouts), and the implementation will be easy to update once mini_layouts has a D10-compatible release. I'll plan to move forward with that approach.
Comment #19
rlmumfordHi Everyone,
Thanks for your work on this. One of my two concerns has been addressed and that was that there were a lot of DocBlock and naming convention changes that were a) out of scope and b) were failing phpcs on Drupal 9.
My second concern is that I want to maintain support for D8.9 and D9.2 because of some legacy projects I maintain that are not able to upgrade right now. What I'll do is make a new release branch for 9.3 up. I hope that meets everyone's need.
Comment #22
rlmumfordThis has been merged into 2.0.x. I will push and alpha release now.
Comment #23
mark_fullmerThank you, Rob!