Skip to content

Conversation

@mukeshpanchal27
Copy link
Member

Summary

Followup work discussed here: #885 (comment)

This PR updates can load infrastructure so that it will support the custom message.

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@mukeshpanchal27 mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release Miscellaneous Issues not related to an existing focus area labels Nov 30, 2023
@mukeshpanchal27 mukeshpanchal27 self-assigned this Nov 30, 2023
@mukeshpanchal27 mukeshpanchal27 marked this pull request as ready for review November 30, 2023 09:47
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @mukeshpanchal27, this looks great. Only a few minor points of feedback on wording and adding a @since annotation for the technically (low-level) breaking change on the function return value.

@felixarntz felixarntz changed the title Update can load infrastructure Allow module can-load.php callbacks to return a WP_Error with more information Nov 30, 2023
@felixarntz felixarntz added this to the PL Plugin 2.8.0 milestone Nov 30, 2023
@felixarntz felixarntz removed no milestone PRs that do not have a defined milestone for release Miscellaneous Issues not related to an existing focus area labels Nov 30, 2023
@felixarntz
Copy link
Member

Milestoning this and #290 for 2.8.0 - not critical, but both of them look very good and we can probably include them in that release. Potentially we can even merge them tomorrow already.

@mukeshpanchal27
Copy link
Member Author

Thanks for the feedback. The PR is ready for final review and merge.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thank you for the updates @mukeshpanchal27, LGTM!

@felixarntz felixarntz merged commit 0e10d5a into trunk Dec 1, 2023
@felixarntz felixarntz deleted the update/can-load-infrastructure branch December 1, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants