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

Single mapImage shouldn’t be optional #1789

Merged
merged 2 commits into from Feb 11, 2019

Conversation

Projects
None yet
4 participants
@bjarkehs
Copy link
Contributor

commented Jan 15, 2019

Unless I misunderstand how RxSwift works then the signature for Single's mapImage is incorrect. The comment also states that if conversion fails, then the signal errors. So the value will always be non-optional.

Response's mapImage also returns non-optional UIImage.

bjarkehs added some commits Jan 15, 2019

@MoyaBot

This comment has been minimized.

Copy link

commented Jan 15, 2019

1 Warning
⚠️ Any changes to library code should be reflected in the Changelog. Please consider adding a note there and adhere to the Changelog Guidelines.
3 Messages
📖 iOS: Executed 254 tests, with 0 failures (0 unexpected) in 11.499 (11.623) seconds
📖 tvOS: Executed 254 tests, with 0 failures (0 unexpected) in 11.171 (11.342) seconds
📖 macOS: Executed 254 tests, with 0 failures (0 unexpected) in 11.332 (11.458) seconds

Generated by 🚫 Danger

@codecov-io

This comment has been minimized.

Copy link

commented Jan 15, 2019

Codecov Report

Merging #1789 into development will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #1789   +/-   ##
============================================
  Coverage        90.83%   90.83%           
============================================
  Files                5        5           
  Lines              131      131           
============================================
  Hits               119      119           
  Misses              12       12
Impacted Files Coverage Δ
Sources/RxMoya/Single+Response.swift 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b791faa...69496e2. Read the comment docs.

1 similar comment
@codecov-io

This comment has been minimized.

Copy link

commented Jan 15, 2019

Codecov Report

Merging #1789 into development will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #1789   +/-   ##
============================================
  Coverage        90.83%   90.83%           
============================================
  Files                5        5           
  Lines              131      131           
============================================
  Hits               119      119           
  Misses              12       12
Impacted Files Coverage Δ
Sources/RxMoya/Single+Response.swift 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b791faa...69496e2. Read the comment docs.

@sunshinejr
Copy link
Member

left a comment

Sorry for late merge & thanks for the work @bjarkehs!👍

@sunshinejr sunshinejr merged commit 8a57552 into Moya:development Feb 11, 2019

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: carthage_without_swiftlint_integration Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.