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

Add function_exists() check to block PHP template #147

Merged
merged 1 commit into from May 8, 2018

Conversation

2 participants
@salcode
Contributor

salcode commented Apr 22, 2018

Check for the existance of the function register_block_type() in the PHP
code when creating a block.

Since Gutenberg has not yet been merged into core, currently the function
register_block_type() is only defined if the Gutenberg plugin is installed
and activated.

See #146

@salcode

This comment has been minimized.

Show comment
Hide comment
@salcode

salcode Apr 22, 2018

Contributor

Reading through the existing Behat tests for blocks, it looks like checks for specific lines of code are only being done on lines that are dynamic (i.e. there is a placeholder that gets replaced). Based on this, I have not added any tests for this change.

If I've misunderstood things, please let me know and I'd be happy to update this PR.

Thanks.

Contributor

salcode commented Apr 22, 2018

Reading through the existing Behat tests for blocks, it looks like checks for specific lines of code are only being done on lines that are dynamic (i.e. there is a placeholder that gets replaced). Based on this, I have not added any tests for this change.

If I've misunderstood things, please let me know and I'd be happy to update this PR.

Thanks.

@schlessera

schlessera requested changes May 8, 2018 edited

No, I think there's no need to add a test for this.

However, given that this is not usual practice, and that the scaffolding tools often serve an educational purpose as well, I'd prefer to have a comment before the function_exists(), along the lines of:

// Skip block registration in case Gutenberg is not enabled/merged.
Add function_exists() check to block PHP template
Check for the existance of the function register_block_type() in the PHP
code when creating a block.

Since Gutenberg has not yet been merged into core, currently the function
register_block_type() is only defined if the Gutenberg plugin is installed
and activated.

See #146
@salcode

This comment has been minimized.

Show comment
Hide comment
@salcode

salcode May 8, 2018

Contributor

@schlessera

Thanks for the feedback. You make a good point that we should call out why this code is included.

I've updated this PR with the following comment as you suggested.

// Skip block registration if Gutenberg is not enabled/merged.

Thanks.

Contributor

salcode commented May 8, 2018

@schlessera

Thanks for the feedback. You make a good point that we should call out why this code is included.

I've updated this PR with the following comment as you suggested.

// Skip block registration if Gutenberg is not enabled/merged.

Thanks.

@schlessera schlessera added this to the 1.1.4 milestone May 8, 2018

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera May 8, 2018

Member

Thanks for the pull request, @salcode !

Member

schlessera commented May 8, 2018

Thanks for the pull request, @salcode !

@schlessera schlessera merged commit 7a9b881 into wp-cli:master May 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment