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.

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

Block library: Extract deprecated field to their own files #15057

Merged
merged 3 commits into from
May 21, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Apr 18, 2019

Description

Part of aligning block library to Block Registration API RFC #13693.

It's all about moving deprecated fields to their own file to follow the proposal drafted in RFC.

Updated blocks:

  • Gallery
  • Image

Removed block deprecations

How has this been tested?

npm test
npm run test-e2e

Manually tested whether all blocks load as before.

@gziolo gziolo added [Package] Block library /packages/block-library [Feature] Block Directory Related to the Block Directory, a repository of block plugins [Type] Task Issues or PRs that have been broken down into an individual action to take labels Apr 18, 2019
@gziolo gziolo self-assigned this Apr 19, 2019
@gziolo gziolo marked this pull request as ready for review April 19, 2019 12:22
@gziolo gziolo requested a review from a team April 19, 2019 12:23
@gziolo gziolo force-pushed the update/block-library-deprecated branch from a5b76bf to 41aae71 Compare April 23, 2019 16:41
@ellatrix
Copy link
Member

It's a bit hard to review this without any tests for deprecated blocks. Would it be possible to add some simple tests to see if deprecated block structures are converted correctly?

@gziolo
Copy link
Member Author

gziolo commented Apr 24, 2019

It's a bit hard to review this without any tests for deprecated blocks. Would it be possible to add some simple tests to see if deprecated block structures are converted correctly?

Good idea. I will add something based on the existing fixtures. I think Columns and Paragraph block have some coverage already:

@gziolo
Copy link
Member Author

gziolo commented Apr 30, 2019

@ellatrix, I opened #15268 with tests covering all possible deprecations. Let's merge it first and then rebase this one.

@gziolo gziolo added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label May 7, 2019
@gziolo
Copy link
Member Author

gziolo commented May 7, 2019

Blocked by #15268.

@gziolo gziolo force-pushed the update/block-library-deprecated branch from 41aae71 to 9f5c46c Compare May 9, 2019 12:58
@gziolo gziolo removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label May 9, 2019
@gziolo gziolo force-pushed the update/block-library-deprecated branch from 9f5c46c to 9fa63c9 Compare May 16, 2019 12:28
@gziolo
Copy link
Member Author

gziolo commented May 16, 2019

Based on my question #15268 (comment):

I tried to add tests for List and Heading blocks but it looks like deprecated versions left for those blocks don't work anymore. It looks like #7349 was added temporary and wasn't planned to be supported for long. Does it mean we can safely remove them?

I can confirm that we those deprecations won't work anymore for List and Heading blocks and can be safely removed. The related logic that handles source: property was removed in https://github.com/WordPress/gutenberg/pull/8276/files#diff-7dae7b3890cf27659680c926b6992e31L108.

@gziolo gziolo added the Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code label May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Directory Related to the Block Directory, a repository of block plugins Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Package] Block library /packages/block-library [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants