Port origin PR labels to backport PR about backport-action HOT 10 CLOSED

Isn't an implementation based on a regex more flexible and similar easy to implement? Because I think there may be some labels we don't want to backport e.g. all our rebuild-xxx labels. I don't think we necessarily need to optimize for ease of use. Flexibility may be more important in my opinion if you want to keep the implementation simple.

from backport-action.

Thanks for getting back to me so fast.

Flexibility wise I think regular expressions are the way to go. To keep complexity in check maybe allow a list of multiple simpler patterns that would each be sufficient for a label to be backported.

Does that sound sensible to you?

from backport-action.

Hi @mweinelt. Thanks for the great idea. I can definitely see your and other use cases for this.

There's some scoping to be done here though. For example, we could define a set of labels that are always set on backport pull requests, but we can also copy labels from the original pull request. Copying also comes with some interesting edges when considering which labels to copy, e.g. simply all labels, or all labels that are not a backport label (so it doesn't trigger the action again), all matching some configurable regex pattern, or all from a configurable set.

I'd be curious to hear from you what would be most fitting to your case, so I can keep the initial change small but also consider how to expand it in the future.

from backport-action.

Hi, I think I'm here for the same reason as mweinelt :)

copy_labels_pattern would be great to have for nixpkgs's use case. Would you accept a PR implementing it?

from backport-action.

@mohe2015 That actually makes sense. Flexibility first, ease of use second.

@lheckemann Let's go for the copy_labels_pattern. To make sure we don't change existing behavior, please use an empty string to mean "don't copy labels using a pattern".

from backport-action.

Fixed in https://github.com/DeterminateSystems/backport-action/commit/eec52aeb846db9a3395467064b78155a4b5b2ddf

from backport-action.

@lheckemann Please note that your code is based on changes that are currently under battle-testing in v1-rc1. They should improve the performance for nixpkgs significantly, but I wanted to ensure they behave correctly before releasing it as v1 (planned for the end of this month). The latest stable release is v0.0.9.

from backport-action.

allow a list of multiple simpler patterns

I'm not sure about this. Regular expressions already support matching to multiple patterns using the |, e.g. abc|def. Do you think another listing syntax would be better suitable? And if so, what should that look like? It's definitely more complex to build, but if there's good reason for it then I wouldn't mind it.

How about naming of the input? What do you think about copy_labels_pattern? It's focused, so we could separate it from a add_labels, which could just be a comma-separated set of labels.

from backport-action.

I think what @mweinelt meant was something like pre-made regex patterns for simple cases that you could use instead of having to spin your own (possibly faulty) regex.

"Every label but backport labels" is probably a property a large amount of users would want, so this common case could be built in as a convenience option. Otherwise, all those users would have to declare a regex for that themselves.

It's a way to have maximum configurability while retaining ease of use which is something we quite like in the functional world ;)

from backport-action.

@lheckemann I would definitely welcome a PR for this, but let's agree first on the design.

We might be able to simplify the feature by copying all non-backport labels. For example, a boolean input copy_labels. When set to true, it copies all labels from the original PR except those that match the label_pattern input.

What do you think?

EDIT: on second thought, that's exactly what @Atemu described 💡 "Every label but backport labels"

from backport-action.