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

Fix deprecated _admin_bar_bump_cb; Fix failing PHP unit tests #7635

Merged
merged 17 commits into from
Oct 10, 2023

Conversation

thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Oct 3, 2023

Summary

Fixes #7619

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added this to the v2.4.3 milestone Oct 3, 2023
@thelovekesh thelovekesh force-pushed the fix/admin-bar-styles branch from 941e2bd to c6717cd Compare October 3, 2023 18:26
@thelovekesh thelovekesh force-pushed the fix/admin-bar-styles branch 2 times, most recently from 0a0fae6 to 30736fe Compare October 6, 2023 18:50
@thelovekesh thelovekesh marked this pull request as ready for review October 10, 2023 16:12
@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2023

Plugin builds for 82fe088 are ready 🛎️!

.github/workflows/build-test-measure.yml Outdated Show resolved Hide resolved
Comment on lines 551 to 561
- name: Override default PHPUnit configuration
if: ${{ matrix.experimental == true && needs.pre-run.outputs.changed-php-count > 0 }}
run: |
cp phpunit.xml.dist phpunit.xml

# Avoid converting deprecations, errors, notices, and warnings to exceptions in experimental mode.
sed -i 's/convertDeprecationsToExceptions="true"/convertDeprecationsToExceptions="false"/g' phpunit.xml
sed -i 's/convertErrorsToExceptions="true"/convertErrorsToExceptions="false"/g' phpunit.xml
sed -i 's/convertNoticesToExceptions="true"/convertNoticesToExceptions="false"/g' phpunit.xml
sed -i 's/convertWarningsToExceptions="true"/convertWarningsToExceptions="false"/g' phpunit.xml
working-directory: ${{ env.WP_CORE_DIR }}/src/wp-content/plugins/amp
Copy link
Member

Choose a reason for hiding this comment

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

😎

includes/class-amp-theme-support.php Outdated Show resolved Hide resolved
Comment on lines -278 to +283
return [ 'file' => $file ];
$file = str_replace( '{{ get_template_directory }}', get_template_directory(), $file );
$file = str_replace( '{{ get_stylesheet_directory }}', get_stylesheet_directory(), $file );

return [
'file' => $file,
];
Copy link
Member

Choose a reason for hiding this comment

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

😎

tests/php/test-amp-helper-functions.php Show resolved Hide resolved
@@ -454,6 +459,7 @@ public function test_delete_transients( $using_ext_object_cache ) {
'_transient_foo',
'_transient_timeout_bar',
'_transient_timeout_baz',
'_transient_wp_theme_patterns_' . get_stylesheet(),
Copy link
Member

Choose a reason for hiding this comment

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

Good that you tested this is preserved.

includes/class-amp-theme-support.php Outdated Show resolved Hide resolved
includes/class-amp-theme-support.php Outdated Show resolved Hide resolved
$this->assertFalse( has_action( 'wp_head', $callback ) );
}

remove_action( 'wp_print_styles', 'print_emoji_styles' );
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a // comment after this and the other instances in this file just to explain why it is removed for future reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done in 82fe088.

Copy link
Member

Choose a reason for hiding this comment

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

In default-filters.php in core now, it has:

add_action( 'admin_print_styles', 'print_emoji_styles' ); // Retained for backwards-compatibility. Unhooked by wp_enqueue_emoji_styles().

So isn't this handled by wp_enqueue_emoji_styles()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is_admin() is false at this point.

unset( $GLOBALS['current_screen'] );

@westonruter westonruter merged commit 92538fb into develop Oct 10, 2023
33 of 34 checks passed
@westonruter westonruter deleted the fix/admin-bar-styles branch October 10, 2023 19:50
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Call to _admin_bar_bump_cb causes deprecation error in WordPress 6.4+
2 participants