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

Blocks: add block registration to package #17167

Merged
merged 13 commits into from
Sep 25, 2020
Merged

Blocks: add block registration to package #17167

merged 13 commits into from
Sep 25, 2020

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Sep 15, 2020 •
edited
Loading

Changes proposed in this Pull Request:

  • This PR adds methods used to register a block.
  • This just adds the methods, we'll then start using them in blocks in a future PR.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

  • N/A

Testing instructions:

  • Not much to test yet, those methods aren't in use just yet.
  • Do the tests pass?

Proposed changelog entry for your changes:

  • N/A

@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] Normal DO NOT MERGE don't merge it! [Package] Blocks labels Sep 15, 2020
@jeherve jeherve self-assigned this Sep 15, 2020
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Sep 15, 2020
@jeherve jeherve mentioned this pull request Sep 15, 2020
Closed
@jeherve jeherve force-pushed the add/blocks-package-registration branch from 8dfb5a5 to 845367e Compare September 15, 2020 16:14
@jeherve jeherve force-pushed the update/blocks-package-new-methods branch from 17e8d4a to f09531a Compare September 16, 2020 09:32
@jeherve jeherve force-pushed the add/blocks-package-registration branch from 845367e to 728b5a9 Compare September 16, 2020 09:57
@jeherve jeherve force-pushed the update/blocks-package-new-methods branch from f09531a to d56a0a9 Compare September 17, 2020 16:31
Base automatically changed from update/blocks-package-new-methods to master September 17, 2020 17:37
@jeherve jeherve force-pushed the add/blocks-package-registration branch from 728b5a9 to 1f7e2a7 Compare September 17, 2020 17:44
@jeherve jeherve removed the DO NOT MERGE don't merge it! label Sep 17, 2020
@jeherve jeherve added this to the 9.0 milestone Sep 17, 2020
@jetpackbot
Copy link

jetpackbot commented Sep 17, 2020 •
edited
Loading

Scheduled Jetpack release: October 6, 2020.
Scheduled code freeze: September 29, 2020

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17167

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Generated by 🚫 dangerJS against 07cf13a

kraftbj
kraftbj previously requested changes Sep 21, 2020
View reviewed changes
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
@kraftbj kraftbj added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 21, 2020
@jeherve jeherve force-pushed the add/blocks-package-registration branch from 1f7e2a7 to 98552cc Compare September 22, 2020 09:30
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 22, 2020
anomiex
anomiex requested changes Sep 22, 2020
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

Seems generally good. Several questions and minor fixes.

packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
packages/blocks/tests/php/test-blocks.php Outdated Show resolved Hide resolved
packages/blocks/tests/php/test-blocks.php Outdated Show resolved Hide resolved
packages/blocks/tests/php/test-blocks.php Outdated Show resolved Hide resolved
@anomiex anomiex added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 22, 2020
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 23, 2020
anomiex
anomiex requested changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

A few more comments on the doc comments. Looks good to me otherwise.

packages/blocks/src/class-blocks.php Show resolved Hide resolved
packages/blocks/src/class-blocks.php Show resolved Hide resolved
packages/blocks/src/class-blocks.php Outdated Show resolved Hide resolved
@anomiex anomiex added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 23, 2020
jeherve and others added 3 commits September 24, 2020 19:05
@jeherve @anomiex
Update packages/blocks/src/class-blocks.php
551da16 
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@jeherve @anomiex
Update packages/blocks/src/class-blocks.php
e2e01fb 
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@jeherve
Refer to originating function in docblock
07cf13a 
See https://github.com/Automattic/jetpack/pull/17167/files#r493670862
@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 24, 2020
@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 24, 2020
@jeherve jeherve dismissed kraftbj’s stale review September 25, 2020 08:20

Changes addressed

@jeherve jeherve merged commit ab9c179 into master Sep 25, 2020
@jeherve jeherve deleted the add/blocks-package-registration branch September 25, 2020 08:21
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Sep 25, 2020
@jeherve jeherve mentioned this pull request Sep 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anomiex anomiex anomiex approved these changes

@kraftbj kraftbj kraftbj left review comments

Assignees

@jeherve jeherve

Labels
[Package] Blocks [Pri] Normal [Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@jeherve @jetpackbot @kraftbj @anomiex @matticbot