chore: describe eslint disables and type the concurrency result buffer - #55
Merged
Conversation
GitHub's type-aware review on main flagged two real nits: the no-control-regex disable directives lacked the required descriptions, and mapWithConcurrency allocated its result buffer as any[] via an untyped new Array(). Add the -- descriptions and use new Array<R>(). Co-authored-by: QoderAI (Qwen 3.8 Max) <qoder_ai@qoder.com>
The stricter type-aware review on main also flags the perf diagnostic console.info and the custom-spawn double assertion. Both are deliberate: the [qoderian perf] lines are the first-turn timing feature's output, and the assertion bridges ChildProcess's nullable streams to the SDK's non-nullable SpawnedProcess. Describe them with disable directives so the intent is explicit instead of silent. Co-authored-by: QoderAI (Qwen 3.8 Max) <qoder_ai@qoder.com>
Enable no-console (allowing warn/error) and @typescript-eslint/no-unnecessary-type-assertion at warn for src, so the disable directives documenting the two deliberate patterns are consumed locally instead of being reported as unused, and local lint stays aligned with the stricter type-aware review on GitHub. Co-authored-by: QoderAI (Qwen 3.8 Max) <qoder_ai@qoder.com>
The stricter review forbids disabling no-console, and the [qoderian perf] console lines were a developer-only nicety. Remove the diagnostics module, its tests, and unwrap every measureAsync/measure/ logElapsed call site back to the direct calls; the performance work itself (parallel metadata reads, probes) stays. Co-authored-by: QoderAI (Qwen 3.8 Max) <qoder_ai@qoder.com>
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
--descriptions to the twono-control-regexeslint-disable directives in the login service (qoder-login-service.ts:46/48), which the type-aware review on main reported as an Error ("Unexpected undescribed directive comment").mapWithConcurrencynow allocates its result buffer withnew Array<R>()instead of the untypednew Array()(any[]), clearing the@typescript-eslint/no-unsafe-assignmentwarning at map-with-concurrency.ts:15.ChildProcess's nullable streams to the SDK's non-nullableSpawnedProcess, a bridge TS cannot infer, so the assertion stays.[qoderian perf]lines): the review forbids disablingno-console, and the lines were a developer-only nicety. The diagnostics module and its two test files are removed and everymeasureAsync/measure/logElapsedcall site is unwrapped back to the direct call; the actual performance work (parallel metadata reads, probes) stays.no-console(allowingwarn/error) and@typescript-eslint/no-unnecessary-type-assertionatwarnforsrcin the local eslint config, keeping local lint aligned with the stricter GitHub review (and consuming the remaining custom-spawn directive locally).Verification
npm run typechecknpm run lint(clean: 0 errors, 0 warnings)npm run test(full suite: 3486/3486 across 160 suites)npm run buildnpm run release:check— not run: no packaging/version changesnpm run audit:prod— not run: no dependency changesSafety
CHANGELOG.md(none in this PR; CHANGELOG untouched)