-
Notifications
You must be signed in to change notification settings - Fork 920
Sticky posts and cornerstone content priorities for llms.txt #22316
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
Sticky posts and cornerstone content priorities for llms.txt #22316
Conversation
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
6b0fb76
to
1829cbd
Compare
Pull Request Test Coverage Report for Build 9c00443b1f830383e79e52807964591ec0d9aa13Details
💛 - Coveralls |
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.
Couple of suggestions
* Returns the most recently modified cornerstone content of a post type. | ||
* | ||
* @param string $post_type The post type. | ||
* @param int $limit The maximum number of posts to return. |
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 can be null.
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.
Improved here.
* | ||
* @return Indexable[] array of indexables. | ||
*/ | ||
public function get_recent_cornerstone_per_post_type( string $post_type, ?int $limit ) { |
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.
I would say for
instead of per
since Per
suggests a list of different post types and this is for one.
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.
Good point, fixed here.
src/llms-txt/infrastructure/markdown-services/content-types-collector.php
Show resolved
Hide resolved
src/llms-txt/infrastructure/markdown-services/content-types-collector.php
Show resolved
Hide resolved
…ne-content-has-priority-for-the-postpagecpt-list
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.
CR + ACC 👍
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
post__not_in
arguments inget_posts()
as that would make the query much slower. Instead, we retrieve 5 posts anyway and manually exclude them if they are already fetched before because they are cornerstone.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Allow search engines to show this content
in search results setting is set tono
Allow search engines to show this content
tono
, edit another post and generate the fileadd_filter( 'Yoast\WP\SEO\should_index_indexables', '__return_false' );
snippet and reset indexables. That way we have disabled indexable creation, so the cornerstone content wont be able to be retrievedAllow search engines to show this content
in search results setting is set tono
Allow search engines to show this content
tono
, edit another page and generate the fileRelevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Post Lists
section of Add content in the llms.txt file #22257UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes https://github.com/Yoast/reserved-tasks/issues/578