Skip to content
This repository was archived by the owner on Jul 3, 2022. It is now read-only.

Comments

Map by KeyPath#212

Merged
Thomvis merged 4 commits intoThomvis:masterfrom
RomanPodymov:master
Feb 18, 2020
Merged

Map by KeyPath#212
Thomvis merged 4 commits intoThomvis:masterfrom
RomanPodymov:master

Conversation

@RomanPodymov
Copy link
Contributor

Hello.
Thank you for BrightFutures.
In this pull request I added func map<U>(_ context: @escaping ExecutionContext, keyPath: KeyPath<Value.Value, U>) -> Future<U, Value.Error>. It is similar to map that you already have, but accepts KeyPath instead of a closure.

@Thomvis
Copy link
Owner

Thomvis commented Feb 16, 2020

Hi, thanks for contributing to BrightFutures!

I like your contribution and I'd love to merge it after discussing one thought I had while going through the changes. Do you think you could leverage the closure-based map implementation to simplify the new map function? So instead of calling into onComplete, call map with a closure that uses the key path. That way we're guaranteed to match the behavior of map.

Let me know what you think!

@RomanPodymov
Copy link
Contributor Author

Hello @Thomvis
I agree with you. Also set !swift(>=5.2) for all new functions because Swift 5.2 provides Key Path Expressions as Functions. testMapByKeyPath is available for all Swift versions.

@Thomvis Thomvis merged commit 6ce6f31 into Thomvis:master Feb 18, 2020
@Thomvis
Copy link
Owner

Thomvis commented Feb 18, 2020

Thanks for making these changes. I've merged the PR! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants