chore(scripts): drop internal tracker refs from comments and docstrings - #213
Merged
Merged
Conversation
Sixteen comments across eight files carried an internal issue identifier that means nothing outside the project, and two module docstrings characterised endpoint behaviour in more operational detail than a fork operator reading the setup scripts needs. This repo is public and every fork operator syncs it, so both are worth not shipping. Replaced with plain descriptions of what the code does. Comment-only apart from three describe() labels. No behaviour change, no control flow touched. 1113 tests pass, check reports 0 errors.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocumentation comments and test suite labels were updated across rate limiting, Turnstile, CSP, and security-header coverage. No runtime logic, API behavior, assertions, or exported interfaces changed. ChangesDocumentation and test label cleanup
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)scripts/setup.tsFile contains syntax errors that prevent linting: Line 656: expected Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/setup.ts`:
- Around line 620-621: Update the comment describing the download-beacon
rate-limit result near the preflight logic to define null as “not attempted”
when there is no domain, no zone, or no token, matching the contract established
around the preflight handling. Keep the existing “error” token-scope explanation
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3851134c-b09e-47cf-843b-c3252a0b4154
📒 Files selected for processing (8)
scripts/apply-download-ratelimit.tsscripts/setup.tsscripts/turnstile-lib.test.tsscripts/turnstile-lib.tsscripts/waf-lib.tssrc/csp-config.test.tssrc/hooks.server.test.tssrc/turnstile-deploy-sync.test.ts
The declaration defines null as not-attempted for no domain, no zone or no token; the Next-steps comment named only the no-zone case, which could read as though a missing domain or token were a token-scope failure.
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.
Comment-only cleanup of the setup/provisioning scripts.
What
Sixteen comments across eight files carried an internal issue identifier that is meaningless outside the project. Two module docstrings also described endpoint behaviour in more operational detail than a fork operator reading the setup scripts needs.
Both replaced with plain descriptions of what the code does. The genuinely useful engineering notes are untouched — the
cf.colo.idrequirement and its API error code, the Free-planmitigation_timeoutconstraint, the widget name-plus-host matching rationale, the idempotency and token-handling notes all stay exactly as they were.Why
This repo is public and every fork operator syncs it, so these comments ship to everyone. Neither category earns its place in a template other people run.
Scope
scripts/setup.tsscripts/turnstile-lib.tssrc/csp-config.test.tsscripts/waf-lib.ts,scripts/apply-download-ratelimit.ts,scripts/turnstile-lib.test.ts,src/hooks.server.test.ts,src/turnstile-deploy-sync.test.tsComment-only apart from three
describe()labels, which are test names rather than logic. No behaviour change, no control flow touched.git difffiltered to non-comment lines returns only those three labels.Verification
npm run check— 0 errorsNote
I ran
prettierover these files at one point, which reformatted them wholesale to its defaults — this repo has no prettier config or format script, so that was wrong. Reset and redone as targeted edits; the diff here is only the intended change.Summary by CodeRabbit
Documentation
Tests