Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete labels feature #183

Merged
merged 3 commits into from
Mar 8, 2023
Merged

Conversation

LiSmi
Copy link
Contributor

@LiSmi LiSmi commented Feb 23, 2023

Resolves #182

This PR introduces an optional delete flag for labels. When true, matching labels will always be deleted. This can be used in conjunction with the allowAddedLabels flag to mark a subset of labels for deletion while leaving others intact, allowing more fine-grained control over label management.

Labels in the local configuration that are missing in the remote will not be created if they are flagged for deletion in the config.

In order to facilitate overriding allowAddedLabels, logic for allowAddedLabels has been moved down into calculateLabelDiff - I considered some alternatives but this felt like the cleanest way to do it, though additional feedback and suggestions are very welcome! Tests and readme have been updated accordingly.

@LiSmi LiSmi requested a review from a team as a code owner February 23, 2023 12:32
@origamiserviceuser origamiserviceuser added this to Backlog in Origami ✨ Feb 23, 2023
@github-actions github-actions bot added the cli Relates to an Origami cli label Feb 23, 2023
@LiSmi LiSmi mentioned this pull request Feb 23, 2023
Copy link
Contributor

@RomuloVitoi RomuloVitoi left a comment

Choose a reason for hiding this comment

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

LGTM.
I've just suggested a few styling changes to keep it consistent with the codebase and setting the delete property default to false

lib/calculate-label-diff.js Outdated Show resolved Hide resolved
lib/calculate-label-diff.js Outdated Show resolved Hide resolved
lib/validate-label-format.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Rômulo Vitoi <romvitoi@gmail.com>
@LiSmi
Copy link
Contributor Author

LiSmi commented Feb 27, 2023

Thanks for taking a look @RomuloVitoi - suggestions committed, good idea there with defaulting to false!

@Eomm
Copy link

Eomm commented Mar 8, 2023

Hi @notlee, any chance to review this PR? 🙏🏼

@notlee notlee added the release:minor Add to a PR to trigger a MINOR version bump when merged label Mar 8, 2023
Copy link
Contributor

@notlee notlee left a comment

Choose a reason for hiding this comment

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

This looks like a great addition to me. Thanks all 👍

@notlee notlee merged commit 3bd1f50 into Financial-Times:master Mar 8, 2023
Origami ✨ automation moved this from Backlog to Done Mar 8, 2023
@notlee
Copy link
Contributor

notlee commented Mar 8, 2023

That's released in 2.3.0 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Relates to an Origami cli release:minor Add to a PR to trigger a MINOR version bump when merged
Projects
Origami ✨
  
Done
Development

Successfully merging this pull request may close these issues.

feat: delete labels
4 participants