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

Can include/exclude regex patterns be handled? #33

Closed
jhlegarreta opened this issue Nov 20, 2021 · 12 comments
Closed

Can include/exclude regex patterns be handled? #33

jhlegarreta opened this issue Nov 20, 2021 · 12 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@jhlegarreta
Copy link

I am wondering whether regex patterns that have both include/exclude parts are correctly handled.

I would like to be able to mark all files whose path matches a pattern with a label, but to avoid apply a different label to files that share only part of the path.

As an example, I have a label named area:Book that I'd like to apply to all *.tex files under the SoftwareGuide/Latex path:

area:Book:
  files:
    - "SoftwareGuide/Latex/*.tex"

At the same time, I have a label named type:BookStyle that should apply to all files under the Latex path, but not to the files under SoftwareGuide/Latex. Using the pattern below:

type:BookStyle:
  files:
    - "Latex/*"

will add both labels to a file like SoftwareGuide/Latex/Appendices/CodingStyleGuide.tex added in a commit.

I am wondering whether modifying the second rule to

type:BookStyle:
  files:
    - "^Latex/[^/]*"

would avoid applying the type:BookStyle label to SoftwareGuide/Latex/Appendices/CodingStyleGuide.tex.

Cross-referencing:
InsightSoftwareConsortium/ITKSoftwareGuide#167

Thanks and sorry to ask a question by opening an issue.

@jhlegarreta
Copy link
Author

Tested in a test repository of my own, and it looks like the mentioned regex pattern is not appropriately dealt with:

 type:BookStyle:
  files:
    - "^Latex/[^/]*"
2021/11/21 00:47:15 Unable to unmarshall legacy config: yaml: unmarshal errors:
  line 3: cannot unmarshal !!seq into labeler.LabelMatcher
  line 5: cannot unmarshal !!seq into labeler.LabelMatcher
  line 7: cannot unmarshal !!seq into labeler.LabelMatcher
  line 9: cannot unmarshal !!seq into labeler.LabelMatcher
  line 11: cannot unmarshal !!seq into labeler.LabelMatcher
  line 15: cannot unmarshal !!seq into labeler.LabelMatcher

Looks like the regex cannot be read properly in here (?):
https://github.com/srvaroa/labeler/blob/master/cmd/action.go#L102

Does addressing this look doable @srvaroa ? Thanks for the effort.

@akutz
Copy link

akutz commented Jan 29, 2023

Mentioned in vmware-tanzu/vm-operator#50 (review) (this was not added automatically for some reason)

@srvaroa srvaroa added enhancement New feature or request help wanted Extra attention is needed labels Feb 11, 2023
@srvaroa
Copy link
Owner

srvaroa commented Feb 11, 2023

Hi, sorry I totally missed this issue. I will take a closer look into this in the coming days. Thanks for reporting.

@srvaroa srvaroa added the question Further information is requested label Feb 11, 2023
@philippwaller
Copy link
Contributor

@jhlegarreta, @akutz: I believe this thread might be helpful to you:

As this GitHub Action is written in Go (Golang), handling regular expressions (Regex) requires a bit of extra care. Specifically, you'll need to escape special characters using double backslashes. This is because the backslash in Go strings is an escape character and therefore must be escaped itself.
Here's an example that should work:

- label: "File"
  files:
    - ".*\\/subfolder\\/.*\\.md"

To clarify this, I've submitted PR (#129) that updates the documentation accordingly.

Originally posted by @philippwaller in #50 (comment)

@srvaroa: Presumably, this is about the same issue.

@srvaroa
Copy link
Owner

srvaroa commented Dec 19, 2023

Yeah, I will add a couple of test cases to prove the examples provided above.

@jhlegarreta
Copy link
Author

@philippwaller thanks for the heads-up; @srvaroa thanks for the documentation; I will try to test it as time permits.

@akutz
Copy link

akutz commented Dec 26, 2023

It is unclear if this actually adds support for excluding file patterns. The negate option does not really work either. We need a way to express:

Apply this label if CONDITION except for GENERATED FILES

Essentially we want to apply labels based on the size of the change, but the size should not be calculated based on changes related to generated content.

@srvaroa
Copy link
Owner

srvaroa commented Dec 27, 2023

@akutz the size condition supports an exclude-files option for this purpose. Right now this only supports explicit exclusions but I could turn it into a regex if that's what you need.

@srvaroa srvaroa self-assigned this Dec 27, 2023
@akutz
Copy link

akutz commented Dec 27, 2023

That would be great, thank you! Please see https://github.com/vmware-tanzu/vm-operator/blob/3ebc8097d391c2356073d7c327b1b0f72d44f5b0/.github/workflows/pr-labeler.yml#L12-L33 for more information.

@srvaroa
Copy link
Owner

srvaroa commented Dec 28, 2023

@jhlegarreta @akutz this should deal with the exclusions in the Size condition. There is an example that should be reasonably understandable in the tests. Let me knot if this looks good for you and I'll get this released asap.

srvaroa added a commit that referenced this issue Dec 28, 2023
Signed-off-by: Galo Navarro <anglorvaroa@gmail.com>
@srvaroa
Copy link
Owner

srvaroa commented Dec 28, 2023

@jhlegarreta I have added a couple of tests to verify the conditions you quoted above. It looks like the expressions work correctly after escaping the characters.

Let me know if that solves your use case.

Tests here -> https://github.com/srvaroa/labeler/pull/132/files#diff-133334d6ef7baf3e8c0bc80d52a8a7c185e1fca8345190d244e4c7c66ee89925R537

srvaroa added a commit that referenced this issue Dec 28, 2023
Signed-off-by: Galo Navarro <anglorvaroa@gmail.com>
@srvaroa
Copy link
Owner

srvaroa commented Dec 28, 2023

I will close this for now, but @jhlegarreta do let me know if your issue is still not solved.

@srvaroa srvaroa closed this as completed Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants