Skip to content

Conversation

wangmengyan95
Copy link
Contributor

  1. Update ParseLogInterceptor to handle GZIP response body stream
  2. Decouple ProxyInputStream (Split the proxy logic and log response logic)
  3. Clean ParseHttpRequest and ParseHttpResponse
  4. Add test

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.update_LogInterceptor_to_handle_gzip_android branch from 4db17a6 to a421eae Compare August 20, 2015 03:43
@wangmengyan95 wangmengyan95 added this to the 1.10.2 milestone Aug 21, 2015
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this to:

public T addHeaders(Map<String, String> headers) {
  this.headers.addAll(headers);
  return self();
}

Leaving it as setHeaders where it can clear all old values seem like it could be confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get around the problem with final and try/catch by using a local var in the constructor and setting the instance vars after the try/catch.

@grantland
Copy link
Contributor

Last nit, just squash and LGTM

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.update_LogInterceptor_to_handle_gzip_android branch from 3b0f8f3 to 0da624a Compare August 28, 2015 18:17
wangmengyan95 added a commit that referenced this pull request Aug 28, 2015
…ceptor_to_handle_gzip_android

Update ParseLogInterceptor to handle gzip stream
@wangmengyan95 wangmengyan95 merged commit 7e0ea0d into master Aug 28, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.update_LogInterceptor_to_handle_gzip_android branch August 28, 2015 18:20
@facebook-github-bot
Copy link

@wangmengyan95 updated the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants