Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
As discussed in slack, because the new normalizer interface requires PHP 8 and there is no simple way to include that while still remaining PHP 7 compatibility due to php linter checks on DrupalCI. See #3248874: Drupal 10 compatibility
As far as I see, the service doesn't even need to be added dynamically, the only dependency on the module is the class name check and if we make that a string then it will just skip over it if the field item class is not that.
Also copied over the test and added a composer.json with a require-dev so the test can run.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | hal-err-3266176-12-interdiff.txt | 4.45 KB | Berdir |
| #12 | hal-err-3266176-12.patch | 14.17 KB | Berdir |
| #9 | hal-err-3266176-9-interdiff.txt | 1.16 KB | Berdir |
| #9 | hal-err-3266176-9.patch | 9.73 KB | Berdir |
| #7 | hal-err-3266176-7-interdiff.txt | 654 bytes | Berdir |
Comments
Comment #2
BerdirComment #3
larowlan+1 - we'll just need to work out testing issues - which are unrelated to this patch.
I've got similar issues in forum too.
Comment #4
BerdirUpdating this, adding the aggregator dev dependency as tests are currently failing on that.
Comment #5
BerdirOoops.
Comment #6
BerdirIgnored composer.json, possibly because I still had the old name in there.
Comment #7
BerdirOk, progress. Fixing that. having weird errors locally but test are actually running now, so lets see.
Comment #9
BerdirThe module has an optional config entity with the same name as the one in rest module, that can't work. I'd propose to just remove it, IMHO having that enabled by default is a bad idea and it doesn't work right now unless you manually delete the rest module default config first. The page cache test is the only test actually relying on that, that's why it didn't work.
And I'd suggest to just remove EntityResourceHalTestCoverageTest, I don't think the goal is to keep enforcing complete test coverage for all core entity types?
Comment #11
larowlanYeh I'm happy to jettison tests we don't need anymore
Comment #12
Berdirwell, need depends on what the goal is. My guess is that it's no longer going to be enforced to have full test coverage for every content entity type in core, it's enough to maintain the test coverage it has.
Did remove that, so this should pass now. Let me know if you'd prefer fixing the existing tests in a separate issue. will be a little bit annoying to reroll as composer.json will be without require-dev but not a big deal.
Comment #14
larowlanCutting a 2.0.1 with this