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

Improve Optimization Detective documentation #1782

Merged
merged 11 commits into from
Feb 11, 2025
Merged

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Jan 9, 2025

This is a follow-up to #1763.

  • Update copy in docs.
  • Remove @access private from interfaces which are intended to be exposed.
  • Flesh out some inline docs.
  • Add missing @since tags.
  • Remove obsolete todos.

See #1857 for follow-up tasks to further flesh out docs.

@westonruter westonruter added [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Documentation Documentation to be added or enhanced labels Jan 9, 2025
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.64%. Comparing base (1f8265d) to head (3f62502).
Report is 12 commits behind head on trunk.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk    #1782   +/-   ##
=======================================
  Coverage   66.64%   66.64%           
=======================================
  Files          88       88           
  Lines        7015     7015           
=======================================
  Hits         4675     4675           
  Misses       2340     2340           
Flag Coverage Δ
multisite 66.64% <ø> (ø)
single 37.27% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@westonruter
Copy link
Member Author

The TODO here can be removed:

* Gets the normalized query vars for the current request.
*
* This is used as a cache key for stored URL Metrics.
*
* TODO: For non-singular requests, consider adding the post IDs from The Loop to ensure publishing a new post will invalidate the cache.
*
* @since 0.1.0
* @access private
*
* @return array<string, mixed> Normalized query vars.
*/
function od_get_normalized_query_vars(): array {

This was solved with the implementation of ETag in #1722.

… update/od-docs

* 'trunk' of https://github.com/WordPress/performance:
  Bump yoast/phpunit-polyfills from 3.1.1 to 4.0.0
  Bump phpstan/php-8-stubs from 0.4.9 to 0.4.10
  Add ETag explainer to od_url_metric_freshness_ttl hook docs
  Fix format for multisite active plugins
  Include active plugins in URL Metric ETag data
  Add missing getters for OD_URL_Metric_Group_Collection props
  Increase default freshness TTL from 1 day to 1 week
@westonruter westonruter marked this pull request as ready for review February 11, 2025 01:08
Copy link

github-actions bot commented Feb 11, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

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.

@westonruter Thanks, LGTM!

@@ -253,6 +253,77 @@ function od_optimize_template_output_buffer( string $buffer ): string {
/**
* Fires to register tag visitors before walking over the document to perform optimizations.
*
* Once a page has finished rendering and the output buffer is processed, the page contents are loaded into
Copy link
Member

Choose a reason for hiding this comment

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

Not opposed to this block for now, but this is so detailed I think it should rather be part of some docs/*.md file. Maybe you can move (most of) it as part of #1857. This is probably relevant for one of the more detailed documentation articles to write, and we wouldn't want to have this much content duplicated between method docs and separate documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, as part of #1857 I'd love to have a process that would automatically extract the docs from phpdoc and dump it into the markdown files. That would ensure the documentation is as close to the code as possible (in PHP) while also being accessible for ease of reading (in Markdown).

Copy link
Member

Choose a reason for hiding this comment

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

While a code reference like that would be helpful, I think large text chunks like this one here still fit better into a more detailed article, e.g. on how to write an Optimization Detective extension.

We'd probably want to avoid duplicating too much between those "manual" articles and automatically generated code reference as well.

@westonruter westonruter merged commit cb79463 into trunk Feb 11, 2025
18 checks passed
@westonruter westonruter deleted the update/od-docs branch February 11, 2025 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Documentation Documentation to be added or enhanced
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants