-
Notifications
You must be signed in to change notification settings - Fork 825
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
Add Glob support for dictionary includes #1477
base: master
Are you sure you want to change the base?
Add Glob support for dictionary includes #1477
Conversation
This change enables the use of Glob expressions (e.g. Sources/*/Package.yml) when including a yml file: ``` include: - path: Sources/*/Package.yml - path: Sources/*/Tests.yml - path: Sources/*/UITests.yml ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would love to see some tests for this in SpecLoadingTests
|
||
private static func includes(from array: [Any], basePath: Path) -> [Include] { | ||
array.flatMap { entry -> [Include] in | ||
if let string = entry as? String, let include = Include(any: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this right only includes defined as a dictionary will be glob enabled, whereas plain strings won't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I wanted to keep the changes as isolated to my use case as possible.
But I can have a look at extending it if you want to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
started working on this, but still ironing out some stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand, I see you've added support for a single string include, but not arrays of strings (where this line is). In breaking 3.0 version, support for these simpler strings could probably be dropped in the future.
Also ideally the glob logic would only be in one place where currently it is split across parse(json:)
and includes(from:,basePath:)
I implemented a very basic test for this |
@yonaskolb Anything I can do here? |
This change enables the use of Glob expressions (e.g. Sources/*/Package.yml) when including a yml file: