Skip to content

refactor(safe-outputs): reduce complexity of execute_impl in submit_pr_review - #1697

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-submit-pr-review-c2dfeceb07ee5111
Draft

refactor(safe-outputs): reduce complexity of execute_impl in submit_pr_review#1697
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-submit-pr-review-c2dfeceb07ee5111

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

execute_impl in src/safe_outputs/submit_pr_review.rs was a 286-line monolithic async function with five inline phases and up to 5 levels of nesting, particularly in the self-approval guard block where an if vote_value > 0 contained nested if pr_response.status().is_success() with another nested if creator_id == Some(user_id).

What changed

Extracted four focused async helper functions, reducing execute_impl to ~100 lines of sequenced calls with early-return guard clauses:

Helper Responsibility
fetch_authenticated_user_id Fetches /connectiondata, extracts authenticated user ID; returns Ok(Err(failure)) on HTTP errors
check_self_approval Positive-vote guard — short-circuits on vote_value <= 0, otherwise fetches PR and returns Some(failure) if caller is the PR author
submit_vote PUTs the review vote to the ADO reviewers endpoint; returns None on success
post_review_comment_thread POSTs the optional comment thread; returns thread ID on success

Verification

  • cargo build — clean
  • cargo test — all tests pass
  • cargo clippy --all-targets --all-features — clean (zero warnings)

Before: 286-line execute_impl, max nesting depth 5
After: ~100-line execute_impl + 4 helpers, max nesting depth 3

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • spsprodeus21.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"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 71.7 AIC · ⌖ 13.6 AIC · ⊞ 7.2K ·

…r_review

Extract four focused helper functions to replace the 286-line monolithic
execute_impl:

- fetch_authenticated_user_id: fetches /connectiondata and extracts the
  authenticated user ID, returning Ok(Err(failure)) on HTTP errors
- check_self_approval: positive-vote guard — returns Some(failure) when
  the caller would vote on their own PR, None otherwise
- submit_vote: PUTs the review vote, returns None on success
- post_review_comment_thread: POSTs the optional comment thread, returns
  the thread ID on success

execute_impl is now ~100 lines of sequenced calls with early-return
guard clauses. Each phase is independently readable and testable.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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.

0 participants