Skip to content

fix: Add consistent debug logging to all branch condition attributes (#1553)#1711

Merged
thomhurst merged 1 commit into
mainfrom
fix/1553-consistent-branch-logging
Jan 1, 2026
Merged

fix: Add consistent debug logging to all branch condition attributes (#1553)#1711
thomhurst merged 1 commit into
mainfrom
fix/1553-consistent-branch-logging

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Add debug logging to branch condition attributes that were missing it
  • All branch condition attributes now log the current branch and the condition being evaluated

Changes

Added debug logging to:

  • RunIfBranchAttribute
  • SkipIfBranchAttribute
  • RunIfBranchStartsWithAttribute
  • RunOnlyIfBranchStartsWithAttribute

This makes them consistent with RunOnlyOnBranchAttribute which already had logging.

Why This Change

When modules are skipped due to branch conditions, having consistent debug logging across all branch condition attributes helps with troubleshooting and debugging pipeline execution.

Fixes #1553

Test plan

  • Build succeeds locally
  • CI builds pass

🤖 Generated with Claude Code

@thomhurst
Copy link
Copy Markdown
Owner Author

Summary

Adds debug logging to Git branch conditional attributes to help troubleshoot branch-based module execution.

Critical Issues

None found ✅

Suggestions

Consider logging at attribute initialization instead of evaluation

The debug logging added to these attributes executes every time the condition is evaluated during the pipeline. This could result in duplicate log entries if the attributes are checked multiple times.

Consider whether this logging belongs in:

  1. The attribute's constructor (logged once when the attribute is created)
  2. A centralized place where all skip/run conditions are evaluated
  3. The module execution framework itself

The current placement is functional but may create verbose logs in pipelines with many conditional modules.

Inconsistent log message wording

  • RunIfBranchAttribute: "Can run on: {ExpectedBranch}"
  • RunIfBranchStartsWithAttribute: "Can run if starts with: {ExpectedPrefix}"
  • SkipIfBranchAttribute: "Will skip on: {SkipBranch}"

These messages have slightly different phrasing. Consider standardizing them for consistency:

  • "Can run on: {Branch}" vs "Will skip on: {Branch}"
  • "Can run if starts with: {Prefix}" vs "Will skip if starts with: {Prefix}"

This would make logs easier to parse and understand at a glance.

Verdict

APPROVE - No critical issues

The changes are straightforward and add helpful diagnostic information. The suggestions above are minor improvements for consideration but don't block the PR.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds debug logging to four branch condition attributes to make them consistent with the existing RunOnlyOnBranchAttribute which already had logging. The changes help with troubleshooting pipeline execution when modules are skipped due to branch conditions.

  • Adds debug logging that displays the current branch and the condition being evaluated
  • Refactors the Condition methods to extract the current branch name into a variable before logging
  • Uses consistent structured logging with template parameters

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/ModularPipelines.Git/Attributes/SkipIfBranchAttribute.cs Adds debug logging showing current branch and the branch that triggers skipping
src/ModularPipelines.Git/Attributes/RunIfBranchAttribute.cs Adds debug logging showing current branch and the expected branch for execution
src/ModularPipelines.Git/Attributes/RunIfBranchStartsWithAttribute.cs Adds debug logging showing current branch and the expected branch prefix for execution
src/ModularPipelines.Git/Attributes/RunOnlyIfBranchStartsWithAttribute.cs Adds debug logging showing current branch and the expected branch prefix for execution

return Task.FromResult(pipelineContext.Git().Information.BranchName != BranchName);
var currentBranchName = pipelineContext.Git().Information.BranchName;

pipelineContext.Logger.LogDebug("Current Branch: {CurrentBranch} | Will skip on: {SkipBranch}", currentBranchName, BranchName);
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The log message "Will skip on: {SkipBranch}" is misleading. This log is emitted every time the Condition method is called, regardless of whether the module will actually skip. The message should clarify what branch pattern causes skipping. Consider changing to "Will skip if on branch: {SkipBranch}" or "Skip condition: branch == {SkipBranch}" to make it clear this is describing the skip condition, not stating that a skip will occur.

Suggested change
pipelineContext.Logger.LogDebug("Current Branch: {CurrentBranch} | Will skip on: {SkipBranch}", currentBranchName, BranchName);
pipelineContext.Logger.LogDebug("Current Branch: {CurrentBranch} | Skip condition: branch == {SkipBranch}", currentBranchName, BranchName);

Copilot uses AI. Check for mistakes.
Add debug logging to branch condition attributes that were missing it:
- RunIfBranchAttribute
- SkipIfBranchAttribute
- RunIfBranchStartsWithAttribute
- RunOnlyIfBranchStartsWithAttribute

All branch condition attributes now log the current branch and the condition
being evaluated, consistent with RunOnlyOnBranchAttribute.

Fixes #1553

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@thomhurst thomhurst force-pushed the fix/1553-consistent-branch-logging branch from 2fe64e7 to b1d4c0d Compare January 1, 2026 16:29
@thomhurst
Copy link
Copy Markdown
Owner Author

Summary

Adds debug logging to Git branch conditional attributes to show current branch and expected branch/prefix during condition evaluation.

Critical Issues

None found ✅

Suggestions

None - the implementation is clean and follows consistent patterns across all four attributes.

Verdict

APPROVE - No critical issues

The debug logging will be helpful for troubleshooting module execution conditions. The structured logging format is correct and the changes are minimal and focused.

@thomhurst thomhurst merged commit 7222d51 into main Jan 1, 2026
12 checks passed
@thomhurst thomhurst deleted the fix/1553-consistent-branch-logging branch January 1, 2026 16:38
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.

Code smell: Inconsistent logging in branch condition attributes

2 participants