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 alamoRequest.cancel() from stubbing closure #1841

Merged
merged 4 commits into from May 1, 2019

Conversation

Projects
None yet
5 participants
@sunshinejr
Copy link
Member

commented Apr 26, 2019

Fixes #1833.

The request was never really resumed and so it most likely doesn't need to be cancelled. This was only making random noise about cancelled request in console.

Targeted master as I'd like to make a 13.0.1 release with this fix.

Remove alamoRequest.cancel() from stubbing closure
The request was never really resumed and so it most likely doesn't need to be cancelled. This was only making random noise about cancelled request in console.
@sunshinejr

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2019

SwiftLint is having problems on newest Xcode/macOS. Wonder if we should try to build it from source or just disable it until it's released as a new version on brew. 🤔

@MoyaBot

This comment has been minimized.

Copy link

commented Apr 27, 2019

2 Warnings
⚠️ Any changes to library code should be reflected in the Changelog. Please consider adding a note there and adhere to the Changelog Guidelines.
⚠️ The library files were changed, but the tests remained unmodified. Consider updating or adding to the tests to match the library changes.
3 Messages
📖 iOS: Executed 278 tests, with 0 failures (0 unexpected) in 13.330 (13.585) seconds
📖 tvOS: Executed 278 tests, with 0 failures (0 unexpected) in 16.597 (16.769) seconds
📖 macOS: Executed 278 tests, with 0 failures (0 unexpected) in 13.414 (13.538) seconds

Generated by 🚫 Danger

@codecov-io

This comment has been minimized.

Copy link

commented Apr 27, 2019

Codecov Report

Merging #1841 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1841      +/-   ##
=========================================
- Coverage   93.48%   93.4%   -0.09%     
=========================================
  Files          24      25       +1     
  Lines         706     864     +158     
=========================================
+ Hits          660     807     +147     
- Misses         46      57      +11
Impacted Files Coverage Δ
Sources/Moya/MoyaProvider+Internal.swift 97% <ø> (+0.7%) ⬆️
Sources/Moya/Plugins/NetworkLoggerPlugin.swift 93.44% <0%> (-6.56%) ⬇️
Sources/ReactiveMoya/SignalProducer+Response.swift 93.93% <0%> (-6.07%) ⬇️
Sources/Moya/MoyaProvider.swift 91.66% <0%> (-2.88%) ⬇️
Sources/Moya/MultiTarget.swift 100% <0%> (ø) ⬆️
Sources/Moya/URLRequest+Encoding.swift 100% <0%> (ø)
Sources/Moya/MultipartFormData.swift 72.41% <0%> (+7.41%) ⬆️

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 08fb6c3...0a04507. Read the comment docs.

@pedrovereza
Copy link
Member

left a comment

🙇

- run:
name: Install Swiftlint
command: brew install swiftlint
command: git clone git@github.com:realm/SwiftLint.git && cd SwiftLint && git submodule update --init --recursive && make install && cd ../

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Apr 27, 2019

Member

Hey :)) Have you considered use Mint here?

This comment has been minimized.

Copy link
@sunshinejr

sunshinejr Apr 28, 2019

Author Member

That's a good point @LucianoPAlmeida. I just didn't want to add another dependency to CI in this PR. We may discuss potential updates to the CI script, as it needs refreshing anyways.

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Apr 28, 2019

Member

Sure, that makes sense, I just raised the point because it appears to be a simpler script :) Also we could avoid having to clone all source and compile it, iff mint supports just download the binary(not sure if it does).

@sunshinejr sunshinejr merged commit 495ef76 into master May 1, 2019

0 of 2 checks passed

ci/circleci: build Your tests are queued behind your running builds
Details
ci/circleci: carthage_without_swiftlint_integration CircleCI is running your tests
Details

@sunshinejr sunshinejr deleted the fix/cancel_notification_on_stubbing branch May 1, 2019

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.