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

Block Hooks API: Update new Navigation functions with @since annotation #58437

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

tjcafferkey
Copy link
Contributor

What?

Update new functions created for Block Hooks API to work with the Navigation block with @since 6.5.0 annotations.

Copy link

Flaky tests detected in dc365aa.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7710802008
📝 Reported issues:

@ockham
Copy link
Contributor

ockham commented Jan 30, 2024

TBH I'm not 100% sure what's our policy with regard to @since annotations, specifically inside dynamic blocks' PHP (i.e. packages/block-library/src/*/*.php). AFAICT, that file currently only has two @sinces, and both are before an apply_filter() (where it makes sense, as those filters become part of Core's API).

It actually gives me pause that none of the functions in the file currently have a @since and makes me wonder if we should even add them to "ours" 🤔 Let's ask @getdave and @youknowriad maybe.

tjcafferkey and others added 2 commits January 30, 2024 14:19
Co-authored-by: Bernie Reiter <96308+ockham@users.noreply.github.com>
@getdave
Copy link
Contributor

getdave commented Jan 30, 2024

I don't see any @since annotations anywhere else...

https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/categories/index.php

@tjcafferkey
Copy link
Contributor Author

@getdave yeah, I checked and noticed this myself but it was explicitly called out as part of this PR, see here https://github.com/WordPress/wordpress-develop/pull/5922/files#r1469818024

I'm unfamiliar with whether it's required/needed or not. If we think not, then we can close the PR.

@getdave
Copy link
Contributor

getdave commented Jan 30, 2024

I did notice a couple of instances in WP Core.

I think perhaps we need to start documenting changes using @since to better align with Core standards. If so then we should try to get folks to prepare for that early - when the change gets merged to Gutenberg.

@youknowriad do you have a view on this? Is there any precedent or prior discussion that I might not be aware of?

@youknowriad
Copy link
Contributor

I think perhaps we need to start documenting changes using @SInCE to better align with Core standards. If so then we should try to get folks to prepare for that early - when the change gets merged to Gutenberg.

I think I agree with this. It's true that we probably didn't follow this consistently so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants