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

any-glob-all-files: combine glob outputs to match all files on all globs #714

Closed
wants to merge 4 commits into from

Conversation

jnewb1
Copy link

@jnewb1 jnewb1 commented Dec 8, 2023

checkout the discussion at #423

there is currently no way to apply the "CI" label with the following rule

any-glob-all-files: [Jenkinsfile, .github/**]

when a PR which changes both Jenkinsfile and github actions files

essentially, this test fails, which to me makes "any glob matches all files" extremely confusing.

describe('check issue #423', () => {
  const changedFiles = ['test.md', 'test.puml'];

  describe('when at least one pattern matches all files', () => {
    const globPatterns = ['*.md', '*.puml'];

    it('returns true', () => {
      const result = checkIfAnyGlobMatchesAllFiles(
        changedFiles,
        globPatterns,
        false
      );
      expect(result).toBe(true);
    });
  });
});

perhaps this behavior should be moved to another rule, since it also breaks this potential use case: https://github.com/actions/labeler/blob/8558fd74291d67161a8a78ce36a881fa63b766a9/__tests__/changedFiles.test.ts#L228C3-L239C6

@jnewb1 jnewb1 requested a review from a team as a code owner December 8, 2023 22:56
@jnewb1 jnewb1 changed the title devcontainer Fix issue #423 Dec 8, 2023
@jnewb1 jnewb1 changed the title Fix issue #423 any-glob-all-files: a more clear implementaion Dec 8, 2023
@jnewb1 jnewb1 changed the title any-glob-all-files: a more clear implementaion any-glob-all-files: combine glob outputs to match all files on all globs Dec 8, 2023
@dfandrich
Copy link
Contributor

dfandrich commented Dec 8, 2023

As mentioned in #423, I'd rather fix any-glob-to-all-files with these new semantics, since I'm pretty sure its current implementation is inadvertently implemented incorrectly. The old semantics are really one-glob-to-all-files but we don't need a special function to do that.

@jnewb1
Copy link
Author

jnewb1 commented Dec 13, 2023

this would be extremely helpful in our repo to get this merged!

@jnewb1
Copy link
Author

jnewb1 commented Dec 15, 2023

@MaksimZhukov

@jnewb1
Copy link
Author

jnewb1 commented Dec 19, 2023

who do I ask for a review to get this merged?

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