-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API Ensure themelist returns an array #612
API Ensure themelist returns an array #612
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer https://github.com/silverstripe/silverstripe-subsites/pull/611/files#r1838742589 as it feels cleaner and ensures we do always set the theme list, whereas it looks like this change could result in having an old not-relevant theme list if $themelist
is null.
That linked solution should still fail as it'll attempt to return null in some circumstances. This unit test started failing after the template work was merged in I think - possibly the linked solution will also fail after rebasing top of the latest work in 6? Could you double check to see if that's the case? |
It's fixing literally this exact same error in that PR, so no it won't still fail. |
Test locally, it does fix it. Will update |
bb73f02
to
e079bda
Compare
6937a2d
to
b4eca2e
Compare
b4eca2e
to
d4bcc9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Issue silverstripe/.github#328
Fixes https://github.com/silverstripe/silverstripe-subsites/actions/runs/11697085928/job/32899670628
1) SilverStripe\Subsites\Tests\SiteTreeSubsitesTest::testThemeResolverIsUsedForSettingThemeList TypeError: SilverStripe\View\SSViewer::set_themes(): Argument #1 ($themes) must be of type array, null given, called in /home/runner/work/silverstripe-subsites/silverstripe-subsites/src/Extensions/SiteTreeSubsites.php on line 405