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

Add missing session headers to Request/NetworkLoggerPlugin. #1878

Merged
merged 8 commits into from Aug 1, 2019

Conversation

@sunshinejr
Copy link
Member

commented Jul 5, 2019

Fixes #1057. Totally forgot about that one and when updating Alamofire I couldn't find the session headers and then I remembered!

This can be reviewed but will be merged only after #1810.


Before:

Moya_Logger: [05/07/2019 12:32:14] Request Headers: [:]

After:

Moya_Logger: [05/07/2019 12:33:50] Request Headers: ["User-Agent": "Basic/1.0 (Moya.Basic; build:1; iOS 12.2.0) Alamofire/5.0.0", "Accept-Encoding": "br;q=1.0, gzip;q=0.9, deflate;q=0.8", "Accept-Language": "en;q=1.0, pl-PL;q=0.9"]

@MoyaBot

This comment has been minimized.

Copy link

commented Jul 5, 2019

1 Warning
⚠️ Consider adding supporting documentation to this change. Documentation can be found in the docs directory.
3 Messages
📖 iOS: Executed 277 tests, with 0 failures (0 unexpected) in 13.441 (13.581) seconds
📖 tvOS: Executed 277 tests, with 0 failures (0 unexpected) in 13.106 (13.272) seconds
📖 macOS: Executed 277 tests, with 0 failures (0 unexpected) in 13.364 (13.491) seconds

Generated by 🚫 Danger

@codecov-io

This comment has been minimized.

Copy link

commented Jul 5, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (development@286d0fe). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##             development    #1878   +/-   ##
==============================================
  Coverage               ?   93.49%           
==============================================
  Files                  ?       26           
  Lines                  ?      907           
  Branches               ?        0           
==============================================
  Hits                   ?      848           
  Misses                 ?       59           
  Partials               ?        0
Impacted Files Coverage Δ
Sources/Moya/Plugin.swift 75% <ø> (ø)
Sources/Moya/RequestTypeWrapper.swift 76.92% <100%> (ø)
Sources/Moya/Moya+Alamofire.swift 85% <100%> (ø)
Sources/Moya/Plugins/NetworkLoggerPlugin.swift 94.02% <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 286d0fe...d15b8bb. Read the comment docs.

output += [format(loggerId, date: date, identifier: "Request Headers", message: headers.description)]
}

if let bodyStream = request?.httpBodyStream {
if let bodyStream = request.request?.httpBodyStream {

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Jul 5, 2019

Member

Nit: request.request can be a little confuse for people reading this later. What you think?

This comment has been minimized.

Copy link
@sunshinejr

sunshinejr Jul 5, 2019

Author Member

Well yeah, this is from the RequestType - we could rename the request to urlRequest maybe. What do you think?

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Jul 5, 2019

Member

Seems nice 👍

}

/// A Nimble matcher that succeeds when at least one of the substrings
public func containOne(of substrings: [String]) -> Predicate<String> {

This comment has been minimized.

Copy link
@LucianoPAlmeida

LucianoPAlmeida Jul 5, 2019

Member

This could be upstreamed to nimble :))
what you think?

This comment has been minimized.

Copy link
@sunshinejr

sunshinejr Jul 5, 2019

Author Member

I was thinking about it, but then I noticed a PR that allows concatenating expectations: Quick/Nimble#493. This way we could introduce or and make it more generic. Already bumped the PR to see if we can get it updated & reviewed.

@sunshinejr sunshinejr changed the base branch from feature/alamofire_5 to development Aug 1, 2019

@LucianoPAlmeida LucianoPAlmeida referenced this pull request Aug 1, 2019

sunshinejr added some commits Aug 1, 2019

Merge branch 'development' into feature/network_logger_headers
# Conflicts:
#	Sources/Moya/Plugins/NetworkLoggerPlugin.swift

@sunshinejr sunshinejr merged commit 6214961 into development Aug 1, 2019

@sunshinejr sunshinejr deleted the feature/network_logger_headers branch Aug 1, 2019

amaurydavid added a commit to amaurydavid/Moya that referenced this pull request Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.