Conversation
|
@getdave Friendly reminder, changes like this need unit tests. Please ensure to backport / add unit tests. |
src/wp-includes/rest-api/endpoints/class-wp-rest-navigation-fallback-controller.php
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
|
All classes, methods and function will need a |
Yep thanks. We have a suite already and I'm working out how/where to port them now. I'm also trying to work out whether this filter makes sense or if there is a centralise point of registration of schemas in Core where I can just inline this. |
This may not be the correct location. Can be moved subject to discussion.
|
|
||
|
|
||
| /** | ||
| * @covers WP_REST_Navigation_Fallback_Controller::get_fallback |
There was a problem hiding this comment.
Unit tests need a ticket number of them.
There was a problem hiding this comment.
...a ticket number of them
I'm not sure what a "ticket number" is. Can you advise further?
There was a problem hiding this comment.
There was a problem hiding this comment.
Thank you for the context.
Use @ticket 12345 to indicate that a test addresses the bug described in ticket...
I read that and I was slightly confused because these files do not fix a bug. However, I assume this is some convention by way that when a backport is introduced we tag the Trac ticket to the test where it was introduced?
If I've got that wrong please do let me know.
audrasjb
left a comment
There was a problem hiding this comment.
I added some docblock related comments.
We can of course fix these small nitpick issues once it's committed, but still better to do it before committing the feature if possible.
I can update the PR accordingly if needed, just let me know.
src/wp-includes/rest-api/endpoints/class-wp-rest-navigation-fallback-controller.php
Show resolved
Hide resolved
src/wp-includes/rest-api/endpoints/class-wp-rest-navigation-fallback-controller.php
Outdated
Show resolved
Hide resolved
|
Looks like I need to add a new mock here 🤔 |
|
I think maybe we just need to regenerate
I'll take a look Edit: trying over here: #4713 Sorry I would have committed to this branch but I have a cheap account 😄 |
|
Thank you!! |
|
Closing in favour of #4713 |
Adds files relating to Navigation fallbacks for the block editor.
Trac ticket: https://core.trac.wordpress.org/ticket/58557
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.