Skip to content

Conversation

@ruwinrmrrr
Copy link
Contributor

@ruwinrmrrr ruwinrmrrr commented Dec 3, 2025

Purpose

This pull request adds more flexibility to how CI triggers are configured in the Azure DevOps build definition module. It introduces new variables that allow you to choose between using YAML-based triggers or specifying custom override settings directly in Terraform. The main changes are grouped below:

CI Trigger Configuration Enhancements:

  • Added a new variable ci_trigger_use_yaml to control whether CI triggers are configured using YAML or custom overrides.
  • Added a new variable ci_trigger_overrides to allow specifying detailed override options for CI triggers, such as batch settings, concurrency limits, polling intervals, and branch/path filters. These overrides are only used when YAML is not selected.

Terraform Resource Logic Updates:

  • Refactored the ci_trigger block in github_build_definition.tf to use dynamic blocks. If ci_trigger_use_yaml is true, a simple YAML-based trigger is used; if false, the trigger is built from the provided overrides, supporting multiple custom configurations.

Summary by CodeRabbit

  • New Features
    • Added flexible CI trigger options for Azure DevOps builds: choose YAML-based triggers or opt into override-based configuration.
    • Override mode lets you configure batching, max concurrent builds per branch, polling intervals, and detailed branch/path include/exclude filters.
    • Introduced a boolean toggle and a structured override input to enable and customize these behaviors.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

Refactors the Azure DevOps build definition's CI trigger: adds a boolean use_yaml and a ci_trigger_overrides list, makes use_yaml drive a conditional use_yaml attribute, and introduces a dynamic override block to populate batch, concurrency, polling, and branch/path filters.

Changes

Cohort / File(s) Summary
CI trigger implementation
modules/azuredevops/GitHub-Build-Definition/github_build_definition.tf
Replaced static ci_trigger config with conditional use_yaml = var.use_yaml == true ? true : null and added a dynamic override block (iterates var.ci_trigger_overrides when var.use_yaml == false) mapping batch, max_concurrent_builds_per_branch, polling_interval, and branch/path include/exclude lists.
Trigger control variables
modules/azuredevops/GitHub-Build-Definition/variables.tf
Added use_yaml (bool, default true) and ci_trigger_overrides (list(object({ batch = bool, max_concurrent_builds_per_branch = number, polling_interval = number, branch_filter_exclude_list = list(string), branch_filter_include_list = list(string), path_filter_exclude_list = list(string), path_filter_include_list = list(string) })), default []).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review dynamic override block mapping to ensure attribute names/types match the Azure DevOps provider.
  • Verify the conditional use_yaml = var.use_yaml == true ? true : null behaves as intended with provider semantics.
  • Validate ci_trigger_overrides object schema and default handling (empty list) in varied use cases.

Poem

🐇 I nibble code beneath the moonlit sky,
Switched blocks that once were rigid, now comply.
YAML or overrides, I choose with glee,
Branches, paths, and polling — hopped for thee.
Pipelines hum; I twitch my whiskers, free.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It covers Purpose well but is missing most required sections: Goals, Approach, User Stories, Release Note, Documentation, Training, Certification, Marketing, Automation Tests, Security Checks, Samples, Related PRs, Migrations, Test Environment, and Learning. Complete the PR description by adding all missing required sections from the template, or indicate 'N/A' with explanation where sections don't apply.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: refactoring CI trigger configuration in the Github Build Definition module, which matches the core objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b5d7ed and 1a4f4c6.

📒 Files selected for processing (2)
  • modules/azuredevops/GitHub-Build-Definition/github_build_definition.tf (1 hunks)
  • modules/azuredevops/GitHub-Build-Definition/variables.tf (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • modules/azuredevops/GitHub-Build-Definition/github_build_definition.tf
  • modules/azuredevops/GitHub-Build-Definition/variables.tf

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
modules/azuredevops/GitHub-Build-Definition/variables.tf (1)

77-89: ci_trigger_overrides structure is well-defined; consider optional naming refinement.

The variable type is correctly specified with clear field semantics. One optional improvement: the _exclude_list and _include_list suffixes are slightly verbose. Consider shortening to _exclude and _include for brevity, provided the list type is clear from the object schema.

Optional refinement for consistency:

  type = list(object({
    batch                            = bool
    max_concurrent_builds_per_branch = number
    polling_interval                 = number
-   branch_filter_exclude_list       = list(string)
-   branch_filter_include_list       = list(string)
-   path_filter_exclude_list         = list(string)
-   path_filter_include_list         = list(string)
+   branch_filter_exclude            = list(string)
+   branch_filter_include            = list(string)
+   path_filter_exclude              = list(string)
+   path_filter_include              = list(string)
  }))

If you apply this refactoring, update the corresponding field references in github_build_definition.tf lines 36–41 to match.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12d788d and 11a8720.

📒 Files selected for processing (2)
  • modules/azuredevops/GitHub-Build-Definition/github_build_definition.tf (1 hunks)
  • modules/azuredevops/GitHub-Build-Definition/variables.tf (1 hunks)
🔇 Additional comments (1)
modules/azuredevops/GitHub-Build-Definition/variables.tf (1)

71-75: Variable definitions look good.

The ci_trigger_use_yaml flag and descriptive comment align well with the intended behavior, and the default of true maintains backward compatibility.

Copy link
Contributor

@SazniMohamed SazniMohamed left a comment

Choose a reason for hiding this comment

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

Address comments

Copy link
Contributor

@SazniMohamed SazniMohamed left a comment

Choose a reason for hiding this comment

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

Added few comments

@SazniMohamed SazniMohamed merged commit 4c6f76a into wso2:main Dec 4, 2025
5 checks passed
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.

2 participants