Skip to content

Fix --discover false enabling recording - #1977

Open
waldekmastykarz wants to merge 1 commit into
dotnet:mainfrom
waldekmastykarz:fix-1957-discover-false
Open

waldekmastykarz wants to merge 1 commit into
dotnet:mainfrom
waldekmastykarz:fix-1957-discover-false

Conversation

@waldekmastykarz

Copy link
Copy Markdown
Collaborator

Closes #1957

--discover false set Record = true because the check was isDiscover is not null. Recording is now enabled only when --discover is true. The logic is extracted to DevProxyCommand.ApplyDiscoverOption and covered by unit tests in DevProxy.Tests.

Only enable recording when --discover is true. Previously any value,
including false, turned recording on.

Closes dotnet#1957

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c4accf12-c9fb-4311-806c-5ed628b79d22
@waldekmastykarz
waldekmastykarz requested a review from a team as a code owner October 21, 2026 13:54
Copilot AI balanced review requested due to automatic review settings October 21, 2026 13:54

Copilot AI 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.

🟢 Approval recommended

The focused fix matches the reported requirements, preserves existing recording settings, and includes regression coverage.

No open issues 🎉

What changed in this PR

Fixes #1957 so --discover false no longer enables recording in Dev Proxy.

Changes:

  • Extracts discovery handling into ApplyDiscoverOption, enabling recording only when discovery is explicitly true.
  • Adds regression tests covering true, false, omitted values, and preservation of existing recording settings.
File Description
DevProxy/​Commands/​DevProxyCommand.cs Corrects and extracts discovery option handling.
DevProxy.Tests/​DevProxyCommandTests.cs Tests discovery’s effect on recording settings.

Review effort: Balanced

Copilot AI 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.

🟢 Approval recommended

The focused fix matches the reported requirements and includes appropriate regression tests, though tests were not executed during review.

No open issues 🎉

Review effort: Balanced

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.

--discover false still enables recording

2 participants