Skip to content

refactor: reduce complexity of execute_add_reviewers in update_pr.rs#1023

Merged
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-update-pr-dcaed729ba50558b
Jun 15, 2026
Merged

refactor: reduce complexity of execute_add_reviewers in update_pr.rs#1023
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-update-pr-dcaed729ba50558b

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

execute_add_reviewers in src/safeoutputs/update_pr.rs (line 648) had a cognitive complexity of 21 (threshold 15 scan; just below the default Clippy threshold of 25). The function combined three separate concerns inline inside a single for reviewer in reviewers loop:

  1. VSSPS identity resolution — a 3-arm match on an HTTP response
  2. Identity unwrapping — a 2-arm match with continue on None
  3. PR reviewer PUT request — another 3-arm match on the HTTP response

The nesting depth drove the complexity score up: loop (1) → identity match (2) → PUT match (3), with each arm adding its own branches.

What changed

Introduced ReviewerAddResult enum — a small private enum with two variants:

  • Added — reviewer was successfully added
  • Failed(String) — short reason string (e.g. "identity not found", "HTTP 404", "request error")

Extracted resolve_and_add_reviewer — a module-level async fn that handles exactly one reviewer:

  • Resolves the ADO identity via VSSPS
  • PUTs the reviewer onto the PR
  • Returns ReviewerAddResult

execute_add_reviewers simplified — the loop body collapses to a single match on ReviewerAddResult, accumulating added/failed lists. All the VSSPS and HTTP logic lives in the extracted helper.

Before / after complexity (at threshold = 15)

Function Before After
execute_add_reviewers 21 < 15 (no warning)
resolve_and_add_reviewer (new) 17

Behaviour preserved

  • Success/failure messages identical (same format strings, same JSON output shape)
  • All 1858 tests pass
  • cargo clippy --all-targets --all-features is clean at the default threshold (25)

Warning

Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com
  • spsprodweu4.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"
    - "spsprodweu4.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 1.4K AIC · ⌖ 34.9 AIC · ⊞ 34.6K ·

Extract per-reviewer loop body into a standalone async helper
resolve_and_add_reviewer, reducing execute_add_reviewers cognitive
complexity from 21 to below 15 (at threshold=15).

The extracted function handles:
- VSSPS identity resolution (3 match arms)
- PR reviewer PUT request (3 match arms)

A new ReviewerAddResult enum carries the per-reviewer outcome back to
the caller, replacing the inline push-to-added/push-to-failed pattern.
Observable behaviour (success/failure messages, JSON output shape) is
unchanged; all 1858 tests continue to pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine jamesadevine marked this pull request as ready for review June 15, 2026 11:10
@jamesadevine jamesadevine merged commit a086791 into main Jun 15, 2026
16 of 20 checks passed
@jamesadevine jamesadevine deleted the refactor/reduce-complexity-update-pr-dcaed729ba50558b branch June 15, 2026 11:12
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