Skip to content

Conversation

@ricky-undeadcoders
Copy link
Contributor

This pull request updates the branch cleanup workflow to improve its handling of dry run logic and prevent failed pipelines when no branches qualify for deletion. It includes 2 changes:

Dry Run Logic Update

Swap the conditional logic for the "Delete branches (dry run)" and "Delete branches" steps in action.yml so that invalid entries will now default to dry-run. For example, consider the following circumstance:

uses: grafana/shared-workflows/actions/cleanup-branches
with:
    dry-run: falses

Currently we would delete the branches, since 'falses' != 'true'. The new logic sees that 'falses' != 'false' and does a dry run.

Handling for No Branches

Added a check to both branch deletion steps to skip execution and print a message if no branches are marked for deletion. Currently it will fail as there is no branches.txt file to parse.

swap handling for inputs.dry-run so that any unknown input value will
default to dry-run
@ricky-undeadcoders ricky-undeadcoders requested a review from a team as a code owner October 9, 2025 19:25
@ricky-undeadcoders ricky-undeadcoders changed the title feat(cleanup-branches): add handling for when no branches qualify for deletion, alter dry-run check logic feat(cleanup-branches): input logic and error handling improvements Oct 9, 2025
@guicaulada
Copy link
Member

guicaulada commented Oct 9, 2025

I find it weird that dry-run is Opt-out instead Opt-in.

Why the choice to go with dry-run as the default?

I would expect:

- uses: grafana/shared-workflows/actions/cleanup-branches

Should just run the clean up branches action and do what it's intended, while now we need to specify:

uses: grafana/shared-workflows/actions/cleanup-branches
with:
    dry-run: false

Dry-run is usually a flag you must opt-in, not opt-out of.

@guicaulada
Copy link
Member

guicaulada commented Oct 9, 2025

Just realized this PR doesn't change this behavior, dry-run was already an opt-out flag since it was true by default, this just makes the behavior more consistent where an explicit, non-typo, false is required for it to opt-out of dry-run, which makes sense. My previous comment is still somewhat valid... but I don't think it's related to this PR anymore. 😅

@ricky-undeadcoders ricky-undeadcoders added this pull request to the merge queue Oct 9, 2025
Merged via the queue into main with commit 2003063 Oct 9, 2025
27 of 28 checks passed
@ricky-undeadcoders ricky-undeadcoders deleted the rwhitaker/cleanup-branches-improvements branch October 9, 2025 20:47
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