Skip to content

Conversation

@flowerthrower
Copy link
Contributor

@flowerthrower flowerthrower commented Nov 10, 2025

Description

Added cpp-linter-ignore-extra input to allow ignoring additional files in C++ linter

Checklist:

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

@flowerthrower flowerthrower self-assigned this Nov 10, 2025
@flowerthrower flowerthrower added the feature New feature or request label Nov 10, 2025
@flowerthrower flowerthrower marked this pull request as ready for review November 26, 2025 09:06
@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Introduces a new optional workflow input cpp-linter-ignore-extra to the reusable C++ linter workflow and updates the linter step to conditionally append pipe-separated extra ignore globs when the input is provided. Documentation (CHANGELOG, UPGRADING) updated to document the input and usage.

Changes

Cohort / File(s) Summary
Workflow Configuration
.github/workflows/reusable-cpp-linter.yml
Added cpp-linter-ignore-extra as a workflow_call input. Updated the Run cpp-linter step to set its ignore argument from a formatted expression that preserves the default ignore pattern and conditionally appends inputs.cpp-linter-ignore-extra prefixed by a pipe when non-empty.
Documentation Updates
CHANGELOG.md
Added an "Added" entry documenting the optional cpp-linter-ignore-extra input (PR #241), an Unreleased upgrade note pointing to UPGRADING.md, a PR reference, and a new contributor entry.
Upgrade Guide
UPGRADING.md
Added [1.17.3] section with "Additional customization for the C++ linter" showing example YAML for cpp-linter-ignore-extra; updated compare/version links.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Workflow caller
    participant Reusable as reusable-cpp-linter.yml
    participant Step as cpp-linter action

    Note right of Reusable: Inputs include default ignore pattern\nand optional `cpp-linter-ignore-extra`
    Caller->>Reusable: invoke workflow (may include cpp-linter-ignore-extra)
    alt extra provided
        Reusable->>Reusable: format ignore = default + "|" + extra
    else none provided
        Reusable->>Reusable: format ignore = default
    end
    Reusable->>Step: run cpp-linter(ignore: formatted ignore)
    Step-->>Reusable: results (success / violations)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the conditional formatting expression handles empty and non-empty inputs.cpp-linter-ignore-extra.
  • Confirm input declaration follows existing workflow_call input conventions and has proper description/type.
  • Check CHANGELOG/UPGRADING text and example YAML for accuracy.

Possibly related PRs

Suggested reviewers

  • burgholzer
  • denialhaag

Poem

🐰 I hopped through YAML, pipe in my paw,
Added a field to ignore what we saw.
Extra globs join the default parade,
C++ linting now better displayed. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Enable ignore by caller inputs' is vague and uses generic phrasing that doesn't clearly describe the specific change—adding cpp-linter-ignore-extra input—making it unclear what the main change entails. Use a more specific title like 'Add cpp-linter-ignore-extra input for customizable ignore patterns' to clearly convey what feature is being added.
Description check ❓ Inconclusive The description is incomplete: the author did not check off tests being added, documentation updates, CI checks, or code review completion; migration instructions are also unchecked despite UPGRADING.md being modified. Complete the checklist by reviewing and checking the applicable items, particularly clarifying whether tests were added, documentation was updated, and CI checks pass.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d04b809 and 522db10.

📒 Files selected for processing (2)
  • CHANGELOG.md (3 hunks)
  • UPGRADING.md (2 hunks)

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.

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

📝 Walkthrough

Walkthrough

A new input parameter cpp-linter-ignore-extra is added to the reusable C++ linter workflow, enabling dynamic ignore patterns. The ignore list computation is updated to conditionally append extra patterns. The changelog documents this feature in version 1.18.0.

Changes

Cohort / File(s) Summary
Workflow Configuration
.github/workflows/reusable-cpp-linter.yml
Adds new input cpp-linter-ignore-extra for pipe-separated ignore globs; updates Run cpp-linter step to dynamically compute ignore parameter using format() that appends extra patterns when provided.
Release Documentation
CHANGELOG.md
Creates new Unreleased section for v1.18.0 with Added entry documenting the cpp-linter-ignore-extra feature; updates version links and adds PR #241 reference.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward addition of a new input parameter with conditional string formatting
  • Documentation-only changes in changelog with standard formatting

Poem

🐰 A linter once rigid, now bends with grace,
Extra ignore patterns find their place,
With pipes and globs, the fuzzy way,
More flexibility blooms today! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Enable ignore by caller inputs' is somewhat vague and doesn't clearly describe the specific change of adding a cpp-linter-ignore-extra input parameter. Consider a more specific title like 'Add cpp-linter-ignore-extra input to reusable workflow' to better clarify what feature is being enabled.
Description check ❓ Inconclusive The description is incomplete; it lacks required sections including motivation/context, dependencies, and most checklist items are unchecked despite work being done. Expand the description to explain why this input is needed, add motivation/context, and accurately reflect which checklist items are completed (mark all completed items).
✅ Passed checks (1 passed)
Check name Status Explanation
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
  • Commit unit tests in branch input_from_caller

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea4fccb and 2eb6b00.

📒 Files selected for processing (2)
  • .github/workflows/reusable-cpp-linter.yml (2 hunks)
  • CHANGELOG.md (4 hunks)
🔇 Additional comments (4)
CHANGELOG.md (2)

12-16: LGTM! Changelog entry is well-formatted and clearly documented.

The new feature is properly categorized, includes example usage in the description, and follows the established pattern with PR and contributor references.


172-173: LGTM! Version links and references are correct.

All version comparison links, PR references, and contributor mentions are properly formatted and consistent with the existing structure.

Also applies to: 194-194, 218-218

.github/workflows/reusable-cpp-linter.yml (2)

49-52: LGTM! New input is well-designed and backward compatible.

The input definition is clear, provides a concrete example of pipe-separated globs, and defaults to an empty string to preserve existing behavior.


159-163: LGTM! Format expression correctly handles dynamic ignore patterns.

The conditional logic properly appends the pipe separator and extra patterns only when non-empty, avoiding duplicate separators or malformed patterns. Base ignores remain unchanged for callers that don't provide the new input.


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.

@burgholzer
Copy link
Member

I also just pushed #255, which I'd like to release today.
I'll check this PR right now, but probably #255 will go in first and this will need to be rebased.

Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Overall LGTM.
This could deserve an entry in the upgrade guide to showcase the new feature.

@burgholzer
Copy link
Member

@flowerthrower #255 just merged. This can be rebased now.

Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Thanks @flowerthrower 🙏🏼
I took the liberty to fix things up here and rebase the PR.
This is all supposed to go into the v1.17.3 release and not yet another patch release.
Should be good to go now.

Copy link
Member

@denialhaag denialhaag left a comment

Choose a reason for hiding this comment

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

This LGTM now. Just two small fixes to the changelog and upgrade guide. 🙂

flowerthrower and others added 2 commits November 26, 2025 14:18
@burgholzer burgholzer merged commit dbcd447 into main Nov 26, 2025
4 checks passed
@burgholzer burgholzer deleted the input_from_caller branch November 26, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants