-
-
Notifications
You must be signed in to change notification settings - Fork 488
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
Docs: Add PostsPerPage XML doc #1732
Conversation
I adapted the wording from the sniff, but adjusted it to remove the reference to high post counts coming from the use of |
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.
Hi @GaryJones Thanks for creating these docs.
I've left some feedback inline.
Aside from that:
- The "valid" and "invalid" code samples don't mirror each other.
What I mean by this is that devs - based on the samples here - could get the impression that using_query_posts()
is always a bad thing, while using$query_args['key']
is always a good thing.
I think having a good/bad example for each way to set the keys would be a good idea. - The second code comparison is missing
<em>
tags for all code samples.
I'm also wondering whether the code comparison split on the key used is actually needed or that these could just be combined into one which shows both keys.
Or maybe better yet: whether there should be separate code comparisons for each way to set the keys. That would mean three code comparisons:
- Array initialization example.
- Adding/overloading a key in an array example.
_query_posts()
example.
@GaryJones Just wondering if you'll have a chance to finish this off in the near future. |
I'm working on this today / tomorrow. |
I've kept them split as is for now, but I'd be OK with merging them into a single valid + invalid comparisons. |
@GaryJones Looks like something went wrong with your cross-branch merging as this PR now seems to contains all the changes since WCEU.... Could you please try to fix this ? If you need help getting the branch cleaned up, please let me know. |
@GaryJones I've had a look at just the doc file, presuming that's the only chance in this PR. Other than my reply about the placement of the info around the default limit, it all looks good to me. Once the branch is fixed, I'm happy to approve and merge this. |
a761e82
to
9eeeb6c
Compare
@jrfnl Rebased into a single commit, and fixed the branch issue - all clean again :-) |
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.
Thanks @GaryJones ! Merging now.
See #1722.