Uh oh!
There was an error while loading. Please reload this page.
register_block_type: Accept editor_script array for back-compat - #3487
register_block_type: Accept editor_script array for back-compat#3487ockham wants to merge 27 commits into
register_block_type: Accept editor_script array for back-compat#3487Conversation
44c2547 to
15dbfb7CompareUh oh!
There was an error while loading. Please reload this page.
gziolo
left a comment
There was a problem hiding this comment.
There is one edge case to address that I explained in https://github.com/WordPress/wordpress-develop/pull/3487/files#r999249106. Otherwise, it's looking good in terms of covering the legacy undocumented behavior. Nice team work on the patch 💯
ockham
commented
Oct 19, 2022
I've added unit test coverage for the issue discussed in the conversation starting at #3487 (comment). They're expected to fail until we settle on a resolution (see #3487 (comment)). |
gziolo
left a comment
There was a problem hiding this comment.
Excellent work. Thank you addressing all my feedback. It wasn’t simple to tackle with different constraints in the REST API and in how the block type was handled in the past. In my eyes, it’s ready to land.
Uh oh!
There was an error while loading. Please reload this page.
Trying to draft a commit message: |
f53fc2f to
7c1c7f7CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
| $this->assertSame( 'hello', $data['editor_script'] ); | ||
| $this->assertSame( 'gutenberg', $data['script'] ); | ||
| $this->assertSame( 'foo', $data['view_script'] ); | ||
| $this->assertSame( 'guten', $data['editor_style'] ); | ||
| $this->assertSame( 'out', $data['style'] ); |
There was a problem hiding this comment.
Test methods with multiple assertions need a message parameter for each assertion, per Core Handbook - Writing PHPUnit Tests - Using Assertions.
ockham
commented
Oct 20, 2022
Thank you very much, @SergeyBiryukov! 🎉 (I think your approval just coincided with my 9bc56a5 which I pushed to address #3487 (comment) -- not sure you saw that commit 😅 ) |
SergeyBiryukov
commented
Oct 20, 2022
Looks even better now 😄 |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…rties Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
…rties, pt. 2 Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
audrasjb
commented
Oct 20, 2022
There's some coding standards issues to fix though :) |
Based on prior work by @nendeb and @costdev in 56707. Quoting that ticket:
Props @nendeb@costdev@gziolo
TODO
Ideally test setter separately from getterProbably okay as-is; they're two faces of the same medal.Trac ticket: https://core.trac.wordpress.org/ticket/56707
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.