-
Notifications
You must be signed in to change notification settings - Fork 385
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
Strip path suffix from pagenum links to fix the posts pagination #5862
Strip path suffix from pagenum links to fix the posts pagination #5862
Conversation
@milindmore22 Would you test this to confirm it is fixing the issue you reported? |
Codecov Report
@@ Coverage Diff @@
## develop #5862 +/- ##
=============================================
+ Coverage 75.15% 75.18% +0.03%
- Complexity 5657 5663 +6
=============================================
Files 210 210
Lines 16987 17009 +22
=============================================
+ Hits 12766 12788 +22
Misses 4221 4221
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Plugin builds for c467d11 are ready 🛎️!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, confirmed issue has been resolved.
@pierlon Ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, one quick question though.
tests/php/src/PairedRoutingTest.php
Outdated
$this->assertFalse( has_action( 'wp_head', 'amp_add_amphtml_link' ) ); | ||
$this->assertEquals( $using_path_suffix, $this->instance->is_using_path_suffix() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check was already done on line 463. Is it necessary once more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no. It was redundant. Removed in c467d11.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done 👌.
Summary
Fixes #5861
Checklist