docs(devlog): publish the remaining Wave 5 record corrections - #1969
Merged
Conversation
I said the alert slipped through because I substituted local tests for missing CI and neither runs CodeQL. True and irrelevant: CodeQL would not have run on #1897 even with full CI, because analyses exist only for main, dev, and PRs targeting the default branch. Every campaign PR targets dev, so none of them could ever get CodeQL feedback - confirmed by the promotion PRs, where the main-targeting pair have analyses and the preview-targeting pair have none. The real cause is duller. Dev is scanned on push and carries 84 open alerts against main's 71. The finding was sitting on the integration branch from the moment #1897 merged until promotion, and no step in this campaign looked at it. Those are two separate holes - CodeQL absent from the dev PR flow, and the post-merge alert list not being anyone's gate - and my first explanation named neither.
…rong PRs Dev reached preview and main through #1962 and #1963, not through the #1958 and #1959 I opened and left for a maintainer - those flipped to merged seconds later once their heads became reachable. So the disclosure I spent three audit rounds getting right sat on the two PRs that moved no code, and the two that actually promoted carried none of it. Fixed by commenting it onto #1962 and #1963 after the fact, which is later than it should have been. Worth naming the failure mode rather than just the fix: I attached the warning to the artifact I controlled rather than to the artifact that would carry the change. js/polynomial-redos is on main now. What I did not do and stand by: I never approved a promotion PR.
The closure-rules section said v2.24.2 while the results table said v2.24.0, and the errata further down already flagged the contradiction. Correcting the line itself rather than leaving a reader to find the footnote: git tag --contains ac8c0d2 names v2.24.0 as the earliest containing tag.
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
documentation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe closure note corrects issue ChangesWave 6 closeout
Estimated code review effort: 1 (Trivial) | ~5 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Three record corrections that existed only locally.
The root cause I recorded for CodeQL #87 was itself wrong. I wrote that merging on local tests plus tsc substituted for CI and skipped static analysis. The real cause is narrower and worse: CodeQL never runs on dev-targeting PRs at all. It scans dev on push and main-targeting PRs, so no per-PR wait would have caught it either way - the alert sat visible among 84 open ones and nobody read them.
The disclosure went to the wrong PRs. I claimed the CodeQL note was in the promotion PR bodies; it was not. It reached #1897 as a follow-up comment, but #1962 and #1963 - the ones an approver reads - never carried it.
The #1843 release version said v2.24.2 in one section and v2.24.0 in another. v2.24.0 is correct, confirmed by git tag --contains ac8c0d2.
Verification
Devlog-only; nothing in the build, typecheck or test path reads devlog/.
Checklist
Summary by CodeRabbit