-
Notifications
You must be signed in to change notification settings - Fork 37
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
Adding hook for custom post status with tests #176
Adding hook for custom post status with tests #176
Conversation
msm-sitemap.php
Outdated
* | ||
* @return string | ||
*/ | ||
public static function get_post_status() { |
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.
public static function get_post_status() { | |
public static function get_post_status(): string { |
I'd suggest adding a type declaration on this, so that any effort to return a non-string is caught earlier before trying to pass it to the SQL query.
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.
Does this method need to be public
?
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.
while it's not required, it does make it much easier to test such as the get_post_stati values mechanism.
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.
Don't mould the API for a class just to make testing either. Non-public methods can be tested via Reflection if necessary.
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Co-authored-by: Gary Jones <[email protected]>
Pull Request: Add
msm_sitemap_post_status
WordPress HookDescription: This pull request introduces a new WordPress hook,
msm_sitemap_post_status
, to facilitate customization of post statuses for inclusion in the sitemap. This feature was specifically requested by a customer to enhance the flexibility of handling custom post statuses in the sitemap functionality.Changes Introduced:
msm_sitemap_post_status
hook inmsm-sitemap.php
.Background: A customer expressed the need for a way to use a post status other than 'published' to build the sitemap, prompting the development of this feature. The introduction of the
msm_sitemap_post_status
hook allows developers to extend the sitemap functionality easily and integrate their custom post statuses.Related Issues:
Reviewers: TBD