Skip to content

refactor: reduce complexity of validate_pr_filters/validate_pipeline_filters in filter_ir.rs - #1708

Merged
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-validate-filters-1e43d1b0a62a17bd
Jul 29, 2026
Merged

refactor: reduce complexity of validate_pr_filters/validate_pipeline_filters in filter_ir.rs#1708
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-validate-filters-1e43d1b0a62a17bd

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

validate_pr_filters (112 lines) and validate_pipeline_filters (46 lines) in src/compile/filter_ir.rs both contained:

  1. Copy-pasted time-window block — the identical 3-check sequence (start format, end format, start == end) appeared verbatim in both functions.
  2. Repeated include/exclude overlap pattern — the find_overlap + if !overlap.is_empty() { diags.push(...) } idiom appeared inline 3 times in validate_pr_filters (for author, build-reason, and twice in labels).
  3. Inline labels block — 3 overlap/empty-check sub-cases nested directly inside validate_pr_filters.

What changed

Extracted three private helpers, keeping public API signatures unchanged:

Helper Purpose
validate_time_window Shared HH:MM format + zero-width window checks; eliminates the copy-paste between the two public functions
check_include_exclude_overlap Single place for the include/exclude conflict pattern, parametric on filter name
validate_labels_filter Encapsulates the three labels sub-checks (any-of∩none-of, all-of∩none-of, empty warning)

Before / After

Function Before After
validate_pr_filters 112 lines, 4 repeated inline patterns, nesting depth 4 ~30 lines, delegates to helpers
validate_pipeline_filters 46 lines, copy-pasted time-window block ~12 lines

Verification

  • cargo build — clean
  • cargo test — all 2580+ tests pass, 0 failures
  • Behaviour is identical — all helpers are pure extractions of existing logic

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 · 116.1 AIC · ⌖ 12.9 AIC · ⊞ 7.2K ·

@azure-pipelines

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

@jamesadevine
jamesadevine marked this pull request as ready for review July 29, 2026 14:09
…filters in filter_ir.rs

Extract three private helpers to eliminate duplicated inline blocks:

- validate_time_window: shared HH:MM + zero-width check used by both
  validate_pr_filters and validate_pipeline_filters (was copy-pasted verbatim)
- check_include_exclude_overlap: single helper for the include/exclude
  conflict pattern repeated for author, build-reason (PR) and build-reason
  (pipeline)
- validate_labels_filter: isolates the three labels sub-checks (any-of ∩
  none-of, all-of ∩ none-of, empty-filter warning)

validate_pr_filters shrinks from 112 lines to ~30; validate_pipeline_filters
from 46 to ~12. Observable behaviour is identical — all tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine
jamesadevine force-pushed the refactor/reduce-complexity-validate-filters-1e43d1b0a62a17bd branch from b18afe3 to a1ea6a2 Compare July 29, 2026 14:09
@azure-pipelines

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

@jamesadevine
jamesadevine merged commit 0ca9783 into main Jul 29, 2026
12 checks passed
@jamesadevine
jamesadevine deleted the refactor/reduce-complexity-validate-filters-1e43d1b0a62a17bd branch July 29, 2026 14:14
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