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

Remove explicit unwrap of optional when looking for framework source #526

Merged
merged 2 commits into from Jul 7, 2015

Conversation

eliperkins
Copy link
Contributor

(Hopefully) Fixes #404

@robrix
Copy link
Contributor

robrix commented Jun 5, 2015

/cc @barksten — any chance you’d be able to give this a test?

@@ -24,11 +24,11 @@ public struct CopyFrameworksCommand: CommandType {
|> flatMap(.Concat) { frameworkPath -> SignalProducer<(), CarthageError> in
let frameworkName = frameworkPath.lastPathComponent

let source = NSURL(fileURLWithPath: frameworkPath, isDirectory: true)!
let source = Result(NSURL(fileURLWithPath: frameworkPath, isDirectory: true), failWith: CarthageError.InvalidArgument(description: "Could not find framework \"\(frameworkName)\" at path \(frameworkPath). Ensure that the given path is approriately entered. Ensure that your \"Input Files\" have been entered correctly."))
Copy link
Member

Choose a reason for hiding this comment

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

I would combine the "Ensure that" sentences. ("…entered and that your…").

Also, there's a typo in appropriately.

@mdiep
Copy link
Member

mdiep commented Jul 5, 2015

This looks good, but unfortunately it conflicts with master now. 😩

If you wouldn't mind updating it, I'd love to merge it! Sorry for the delay!

@eliperkins eliperkins force-pushed the feature/remove-explicit-unwrap branch from c7c942f to 130bade Compare July 6, 2015 20:54
let target = frameworksFolder().map { $0.URLByAppendingPathComponent(frameworkName, isDirectory: true) }

return SignalProducer(result: target &&& validArchitectures())
|> flatMap(.Merge) { (target, validArchitectures) -> SignalProducer<(), CarthageError> in
return combineLatest(SignalProducer(result: source), SignalProducer(result: target), SignalProducer(result: validArchitectures()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After checking out antitypical/Result#47 and #547, I swapped this back to a combineLatest to handle the next value coming along. Using &&& created a tuple within a tuple instead of a tuple with three values.

Not sure if this is best! @robrix @ikesyo

@eliperkins
Copy link
Contributor Author

🐎

@mdiep
Copy link
Member

mdiep commented Jul 7, 2015

Looks great—thanks! ✨

mdiep added a commit that referenced this pull request Jul 7, 2015
Remove explicit unwrap of optional when looking for framework source
@mdiep mdiep merged commit a04ce6f into Carthage:master Jul 7, 2015
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

3 participants