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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude multiple paths in a glob #937

Conversation

maciejpiotrowski89
Copy link
Contributor

@maciejpiotrowski89 maciejpiotrowski89 commented Jan 27, 2020

Short description 馃摑

Prior to the change it was possible to exclude a single source file/glob pattern from a target (#808, #913). The change introduces a way to exclude multiple files/globs from a target:

let glob = SourceFileGlob("Files/**", excluding: ["Sources/**/*Tests.swift", "Sources/**/*+Fake.swift"])

Solution 馃摝

Initializer of SourceFileGlob now takes an array of items to be excluded. The signature of old initializer was left for backward-compatibility.

@maciejpiotrowski89 maciejpiotrowski89 changed the title Feature/exclude multiple paths in a glob Exclude multiple paths in a glob Jan 27, 2020
var excluded: [AbsolutePath] = []
source.excluding?.forEach { path in
let absolute = AbsolutePath(path)
let globs = AbsolutePath(absolute.dirname).glob(absolute.basename)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: you could use flatMap and simply return the array of paths created here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that reduce would suit better, because for every item of the excluding array another array of globs is generated, which needs to be appended to the excluded array.

As a newcomer to the project it took my a while to understand what each type in the transformation is, so I decided to use the code as you see, without flat mapping - reduce magic 馃敭.

@maciejpiotrowski89 maciejpiotrowski89 force-pushed the feature/exclude-multiple-paths-in-a-glob branch from d4ec3b0 to 0180d6f Compare January 27, 2020 15:59
Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

Congrats on your first contribution @paciej00. Changes look good to me. There are some minor bits remaining:

  • Update the CHANGELOG.
  • Solve git conflicts.
  • Solve the swiftformat issues.

@maciejpiotrowski89
Copy link
Contributor Author

@pepibumur thanks !

I updated the CHANGELOG yesterday, however there is a problem with GITHUB_TOKEN I believe https://github.com/tuist/tuist/pull/937/checks?check_run_id=413417427:

Run peterjgrainger/action-changelog-reminder@v1
  env:
    GITHUB_TOKEN: ***
##[error]Resource not accessible by integration
##[error]Node run failed with exit code 1

Is there a way to merge tuist: master into this branch on my fork?

@pepicrft pepicrft merged commit 2096976 into tuist:master Jan 28, 2020
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.

None yet

4 participants