Skip to content

refactor(cli): proxy failure wording without added comments or a try/catch - #6715

Open
miguel-heygen wants to merge 1 commit into
mainfrom
fix/cli-capture-proxy-no-comments
Open

miguel-heygen wants to merge 1 commit into
mainfrom
fix/cli-capture-proxy-no-comments

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changed

Follow-up to the merged capture proxy message: the two docblocks and the // expected: comment in navigateForCapture.ts are removed, and the empty try/catch around the address parse becomes URL.canParse. The messages and behavior are unchanged.

Why

The Comments workflow's comment ratchet reported the file's comment share going from 0 to 1.4% on the merged change and asked for the lines to go, not to be shorter.

Testing

navigateForCapture.test.ts passes (37 tests) with the change; format and lint are clean on the file.

no behavior change

fails without the fix: not a behavior change; the Comments workflow is the check.

@github-actions

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 271 (base branch 271), smooth 154 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (2)

  • seqplainkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0, drop 0, reload 0, render 0.03, renderKey -, undo true, teleport true
  • seqplainkeys-none-pct-r0-nested-z100: tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.05, pressJump 0, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE · 0 blocker · 0 important · 0 nits · head df71defe

URL.canParse gives the same host-or-"the site" result as the removed try/catch, and the CLI requires Node ≥22, which has it. CI, Comments and Windows render are green on this head. This is a refactor, not a fix, so it needs no fails-without test.

— Genesis

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant