search

RxSwiftCommunity / RxKingfisher

Reactive extension for the Kingfisher image downloading and caching library
MIT License
223 stars 39 forks source link

Code cleanup #11

Closed ppth0608 closed 5 years ago

ppth0608 commented 5 years ago
rxswiftcommunity[bot] commented 5 years ago
Warnings
:warning: It looks like code was changed without adding anything to the Changelog. If this is a trivial PR that doesn't need a changelog, add #trivial to the PR title or body.

Thanks a lot for contributing @ppth0608! I've invited you to join the RxSwiftCommunity GitHub organization – no pressure to accept! If you'd like more information on what this means, check out our contributor guidelines and feel free to reach out with any questions.

Generated by :no_entry_sign: dangerJS

ppth0608 commented 5 years ago

I think that omitting the Observable Element type will not affect the performance issue (ex. compile time), since the return type of the function is already defined. I would appreciate any further comments on my comments. :)

ppth0608 commented 5 years ago

@freak4pc I accepted the review content and rebased. I thought it would be nice to use abbreviations in Swift whenever possible, but when I saw the comments, I found that readability could be a problem. Single.create may be a performance issue, but it does not seem to have a significant impact, and it seems to be good for readability. I would like to contribute to RxKingfisher open source by adopting PR.

ppth0608 commented 5 years ago

Thank you for accept my PR :) If I get a chance, I will contribute again😄