Repository navigation
chore: land two self-review nits that missed the 0.51.0 cut - #123
Merged
Merged
Conversation
Both were found in my own review of #122 and both were fixed, tested — and then not committed before the merge. Neither is functional, which is why no release is being re-cut for them: - `isIndex` was declared after `sitemap.isIndex` had already been used a few lines above. Not a temporal-dead-zone error, since the earlier line reads the property rather than the binding; just two ways of asking the same question in one function. - `page_age_negative` sat in the metric-coverage test's NOT_CHARTED list while traffic.js does read it. A waiver list is only worth having if every entry in it is true, or it stops being read as a set of decisions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the 'prerender_ops.page_age_negative' metric from the NOT_CHARTED map in the admin assets test. It also refactors PrerenderAdmin.js by hoisting the isIndex variable declaration so that it can be reused earlier in the method, eliminating a duplicate declaration. There are no review comments, and I have no additional feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both were findings from my own review of #122, fixed and tested there — and then not committed before the merge. Neither is functional, so no release is being re-cut; they will ride the next one.
isIndexwas declared aftersitemap.isIndexhad already been used a few lines above. Not a TDZ error — the earlier line reads the property, not the binding — just two ways of asking the same question inside one function.page_age_negativesat in the metric-coverage test'sNOT_CHARTEDlist whiletraffic.jsdoes read it. A waiver list is only worth having if every entry in it is true, or it stops being read as a set of decisions and becomes a place things go to be forgotten.Plugin 776 and console 136 green, lint + format clean.