Audit e2e/ and playwright.config.ts comments (#225 Phase 2)#244
Merged
Conversation
- e2e/index.spec.ts: delete self-evident comments, convert rationale to WHY: - e2e/prefabs.spec.ts: delete self-evident comments, convert rationale to WHY: - playwright.config.ts: move workers rationale to JSDoc - Remove empty EXCLUDED_PATHS and isExcludedFile from lint plugin
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.
Summary
Phase 2 for #225: audit all of
e2e/andplaywright.config.ts(3 files, 25 flagged comments). Drops the 3 corresponding entries fromEXCLUDED_PATHSand removes the now-empty exclusion mechanism entirely.Coverage
e2e/index.spec.ts(18) — Deleted self-evident comments (Prefab list is populated.,Map name input reflects the selection., step labels likeApply the "trader" prefab-filter preset.). Converted design-rationale comments toWHY:markers: async select population by LabelHandler, bundled-map async population, MutationObserver latch rationale, cold-worker timeout, canvas dimension derivation, long-task isolation from load overhead, threshold tolerances for CI variance, clipboard spy motivation. Collapsed multi-line observer explanation into a single blockWHY:.e2e/prefabs.spec.ts(6) — Deleted self-evidentWait for at least one filtered result.. Converted worker-poll and UrlState-debounce comments toWHY:. Collapsed 7-line XSS regression explanation into a single blockWHY:covering the inability to plant malicious prefabs and the<mark>verification strategy.playwright.config.ts(1) — Moved theworkersrationale (bundled-map load size, server contention) from inline line comments to JSDoc.tools/lint-plugins/require-comment-rationale.ts& test — Removed the emptyEXCLUDED_PATHSarray,isExcludedFilehelper, and its import in the test file, along with the now-obsolete "skips files matching excluded path substrings" test case.shouldPreserveis no longer exported.Style
Single-clause
WHY:markers usingso/because/sincefor causation. No;,—, or colon-elaboration in prose. JSDoc bodies split into short sentences with period.Test plan
deno task ci:lintpassesdeno task ci:fmt-checkpassesdeno task ci:type-checkpassesdeno task test— 41 tests / 240 steps passProgress
EXCLUDED_PATHS: 3 → deleted. The entire codebase is now audited and the exclusion ratchet is removed.