-
Notifications
You must be signed in to change notification settings - Fork 823
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
Support optional include #712
base: master
Are you sure you want to change the base?
Conversation
$0.context("with optional include for non exist subspecs") { | ||
$0.it("ignore subspecs") { | ||
let path = fixturePath + "optional_include/optional_include.yml" | ||
let project = try XCTUnwrap(loadSpec(path: path)) |
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.
XCTUnwrap
will work only on Swift 5.1 or above.
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.
Can you use the new unwrap
function from recently merged #714
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.
fixed
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.
Great addition 👏
$0.context("with optional include for non exist subspecs") { | ||
$0.it("ignore subspecs") { | ||
let path = fixturePath + "optional_include/optional_include.yml" | ||
let project = try XCTUnwrap(loadSpec(path: path)) |
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.
Can you use the new unwrap
function from recently merged #714
Sources/ProjectSpec/SpecFile.swift
Outdated
return try SpecFile(include: include, basePath: basePath, relativePath: relativePath) | ||
} catch { | ||
if include.optional { | ||
return nil |
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.
This would make an optional includes silently fail if there was a different error than simply a missing file, for example if one of the includes own includes was missing. Is that the intention? Maybe it's better to only return nil if the file is actually missing
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.
Maybe it's better to only return nil if the file is actually missing
Do you mean optional
should ignore throwing from try path.read()
?
let data: Data = try path.read() |
@yonaskolb Following your comments I implemented a new strategy to cause errors. However, prior implementation seems to be simpler... |
do { | ||
data = try path.read() | ||
} catch { | ||
if shouldIgnoreNotFound { |
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.
I was thinking throwing a GenerationError.includeNotFound
, which you can then catch at the top level, and it it's not optional then rethrow
Motivation
I want to add targets dynamically. including subspecs help me in the situation.
The included spec is automatically generated and if not exists ignore adding targets.
Description
I added
optional
option toInclude
.This value indicates whether ignoring failure loading (e.g. the path does not exist).