Skip to content

Silverstripe 6 compatibility#79

Merged
thats4shaw merged 7 commits intomainfrom
silverstripe6
Aug 6, 2025
Merged

Silverstripe 6 compatibility#79
thats4shaw merged 7 commits intomainfrom
silverstripe6

Conversation

@TheSceneman
Copy link
Copy Markdown
Collaborator

This PR replaces deprecated Sillverstripe classes with their new namespaces.

It also updates the PHPUnit to version 11 to be in-step with the CMS. This includes a number of changes such as data providers needing to be static and annotations uses rather than @metadata.

Tests passing locally on a SS6 install. Cursory functional testing is good (tested key generation on some pages, and global cares on SiteConfig)

@TheSceneman TheSceneman requested a review from chrispenny June 12, 2025 01:43
@chrispenny
Copy link
Copy Markdown
Member

@TheSceneman the changes look good, but we'll need to figure out what the issue with PHP 8.3 is. I would suggest hitting up the product team about this one:

PHP Fatal error:  Non-readonly class SilverStripe\Dev\Constraint\ModelDataContains cannot extend readonly class PHPUnit\Framework\Constraint\Constraint in /home/runner/work/keys-for-cache/keys-for-cache/vendor/silverstripe/framework/src/Dev/Constraint/ModelDataContains.php on line 15

Comment thread phpcs.xml.dist
<description>CodeSniffer ruleset for SilverStripe coding conventions.</description>

<file>src</file>
<file>tests</file>
Copy link
Copy Markdown
Collaborator

@satrun77 satrun77 Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the best option, but PHPCS is failing to understand #[DataProvider('readingModes')].

Unable to find a solution for it. This can be rewritten without the PHP Attributes or exclude tests from PHPCS until it supports it!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be fine with tests being excluded from PHPCS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🙏 Who can merge and tag a release?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone in the Terraformers team, or me next month once I'm (slightly) freed from my current deadlines.

@thats4shaw @mfendeksilverstripe if anyone has someone available to test/review/etc.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satrun77 @chrispenny I've got some time blocked out next week to get this sorted out.

Apologies for the delay here, the team was been slammed.

@obj63mc
Copy link
Copy Markdown

obj63mc commented Aug 6, 2025

Checking if we could get this merged and released @thats4shaw or @mfendeksilverstripe

Copy link
Copy Markdown

@thats4shaw thats4shaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the nudge @obj63mc.

Everything seems to be working nicely from my testing. Nice work @TheSceneman and @satrun77!

@thats4shaw thats4shaw merged commit aa636dd into main Aug 6, 2025
18 checks passed
@thats4shaw thats4shaw deleted the silverstripe6 branch August 6, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants