search

SvanBoxel / delete-merged-branch

No more manually deleting merged branches, this lovely app does it for you.
https://github.com/apps/delete-merged-branch/
ISC License
324 stars 55 forks source link

WIP: Whitelist target branches for deletion #124

Closed linkdd closed 4 years ago

linkdd commented 4 years ago

If the PR's target branch is not listed in the whitelist, the source branch will not be deleted.

SvanBoxel commented 4 years ago

Thanks for your PR @linkdd!

I'm thinking of what would be the right pattern to use here. We already have an exclude option which implicitly suggests that excluding is enabled. Shouldn't we do the same with whitelisting, and then rename it to included? Similar to the syntax that is used in Azure DevOps (and other CI systems) for defining what branches should trigger CI. https://docs.microsoft.com/en-us/azure/devops/pipelines/build/triggers?view=azure-devops&tabs=yaml

linkdd commented 4 years ago

The modification is done, thank you for the quick reply :)

SvanBoxel commented 4 years ago

Have you thought about the behavior if a branch is both included and excluded? Are they mutually exclusive and should it be handled as an exception or what do you think?

Also, could you update the specs and documentation? Let me know if you need help! Thanks again.

linkdd commented 4 years ago

exclude excludes deletion based on the source branch of the PR. What I wanted with the whitelist (include) was to enable deletion based on the target branch of the PR.

It might be interesting to expose the workflow at my current workplace:

We would like to automatically delete the (fix|feature)/* branch only when it is merged in master.

I will gladly update the specs and documentation once we agree on the code :)

linkdd commented 4 years ago

We could do the following:

patterns:
  - target: master
    delete_on_close: yes
  - source: *
    delete_on_close: no

Where the first matching pattern is applied.