Skip to content

Conversation

wangmengyan95
Copy link
Contributor

No description provided.

@grantland grantland added this to the 1.10.2 milestone Sep 11, 2015
@grantland
Copy link
Contributor

Don't we need to publicize ParseNetworkInterceptor.Chain as well?

Should we make ParseHttpRequest, ParseHttpResponse, their Builders, and Method final or is there a reason they'd be subclassed?

@wangmengyan95
Copy link
Contributor Author

ParseNetworkInterceptor.Chain is public since ParseNetworkInterceptor is public. The compiler recommends to remove public.

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.public_ParseHttpRequest_ParseHttpResponse_and_ParseNetworkInterceptor branch from 2ea9769 to b11a547 Compare September 11, 2015 02:36
@wangmengyan95
Copy link
Contributor Author

Method is enum, can not be final.

@grantland
Copy link
Contributor

Still missing JavaDoc on getters/setters as well as interface methods.

We might as well move these public APIs to com.parse.http since they're all public and don't need private access to anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

{@code } contentType and contentLength

@grantland
Copy link
Contributor

Some small nits, but LGTM. Just fix the nits, squash, and feel free to merge.

  • {@code <blah>} things like -1, Content-Type (ParseHttpBody)

@wangmengyan95 wangmengyan95 force-pushed the wangmengyan.public_ParseHttpRequest_ParseHttpResponse_and_ParseNetworkInterceptor branch from 01c8efc to 0072ae4 Compare September 11, 2015 23:23
wangmengyan95 added a commit that referenced this pull request Sep 11, 2015
…tpRequest_ParseHttpResponse_and_ParseNetworkInterceptor

Public Interceptor related classes
@wangmengyan95 wangmengyan95 merged commit 6d51a01 into master Sep 11, 2015
@wangmengyan95 wangmengyan95 deleted the wangmengyan.public_ParseHttpRequest_ParseHttpResponse_and_ParseNetworkInterceptor branch September 11, 2015 23:27
@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