Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MNT Make data providers static #11557

Merged
merged 1 commit into from
Jan 15, 2025
Merged

MNT Make data providers static #11557

merged 1 commit into from
Jan 15, 2025

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Jan 14, 2025 •
edited
Loading

Issue silverstripe/.github#357

Unit tests originally targeted CMS 5.3, was merged up

This was referenced Jan 14, 2025
Open
Closed
Closed
@emteknetnz emteknetnz force-pushed the pulls/6.0/static-providers branch from 44725d3 to 227bb5e Compare January 14, 2025 23:08
@emteknetnz emteknetnz marked this pull request as ready for review January 15, 2025 00:01
GuySartorelli
GuySartorelli requested changes Jan 15, 2025
View reviewed changes
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Need to change @dataProvider to new syntax as well.

Probably update the commit message to MNT Fix dataprovider syntax too since it's not just making those functions static, but I won't halt merging if that's not changed.

@emteknetnz emteknetnz force-pushed the pulls/6.0/static-providers branch from 227bb5e to 1e47f0a Compare January 15, 2025 00:06
@emteknetnz
Copy link
Member Author

Updated

@GuySartorelli
Copy link
Member

Two relevant errors in CI:

1) SilverStripe\Forms\Tests\FormMessageTest::testGetMessage
ArgumentCountError: Too few arguments to function SilverStripe\Forms\Tests\FormMessageTest::testGetMessage(), 0 passed in /home/runner/work/silverstripe-framework/silverstripe-framework/vendor/phpunit/phpunit/src/Framework/TestCase.php on line 1235 and exactly 4 expected

/home/runner/work/silverstripe-framework/silverstripe-framework/tests/php/Forms/FormMessageTest.php:74

2) SilverStripe\View\Tests\Shortcodes\EmbedShortcodeProviderTest::testSandboxHtml
ArgumentCountError: Too few arguments to function SilverStripe\View\Tests\Shortcodes\EmbedShortcodeProviderTest::testSandboxHtml(), 0 passed in /home/runner/work/silverstripe-framework/silverstripe-framework/vendor/phpunit/phpunit/src/Framework/TestCase.php on line 1235 and exactly 6 expected

/home/runner/work/silverstripe-framework/silverstripe-framework/tests/php/View/Shortcodes/EmbedShortcodeProviderTest.php:369

@emteknetnz emteknetnz force-pushed the pulls/6.0/static-providers branch from 1e47f0a to 7befdad Compare January 15, 2025 00:28
@emteknetnz emteknetnz force-pushed the pulls/6.0/static-providers branch from 7befdad to c307ecc Compare January 15, 2025 00:30
GuySartorelli
GuySartorelli approved these changes Jan 15, 2025
View reviewed changes
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

LGTM

@GuySartorelli GuySartorelli merged commit b842972 into silverstripe:6.0 Jan 15, 2025
12 checks passed
@GuySartorelli GuySartorelli deleted the pulls/6.0/static-providers branch January 15, 2025 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuySartorelli GuySartorelli GuySartorelli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emteknetnz @GuySartorelli