Skip to content

Android WebSocket: include cookies in request - #9114

Closed
antoinerousseau wants to merge 1 commit into
react:masterfrom
antoinerousseau:android-websocket-cookies
Closed

Android WebSocket: include cookies in request#9114
antoinerousseau wants to merge 1 commit into
react:masterfrom
antoinerousseau:android-websocket-cookies

Conversation

@antoinerousseau

Copy link
Copy Markdown
Contributor

This PR updates #6851 from @srikanthkh, fixing coding conventions and javadoc, and adding a test plan.

Test plan

Added testing functions into the WebSocketExample page of the UIExplorer, including a tiny http server to set a cookie on demand. Instructions included in the UIExplorer app.

@ghost

Copy link
Copy Markdown

By analyzing the blame information on this pull request, we identified @philikon and @AndrewJack to be potential reviewers.

@ghostghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 30, 2016
@zxcpoiu

Copy link
Copy Markdown
Contributor

@facebook-github-bot label Android

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 31, 2016
@shirou

Copy link
Copy Markdown

Hi, is there any update? I met same issue and resolved with this great PR.

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 12, 2016
@ghost

Copy link
Copy Markdown

@antoinerousseau updated the pull request.

@antoinerousseau

Copy link
Copy Markdown
ContributorAuthor

@shirou I wish someone from FB would realize that this PR is ready to be merged!

@ghost

Copy link
Copy Markdown

@antoinerousseau updated the pull request.

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 12, 2016
@ghost

Copy link
Copy Markdown

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @philikon as a potential reviewer. Could you take a look please or cc someone with more context?

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 11, 2016
@shirou

Copy link
Copy Markdown

@antoinerousseau Could you update this branch?

@philikon Could you review and if there are no problem, merge this PR? or if you have some concern, I will help. We really need this PR.

Thank you.

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 16, 2016
@ghost

Copy link
Copy Markdown

@antoinerousseau updated the pull request - view changes

@ghostghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 16, 2016
@facebook-github-bot

Copy link
Copy Markdown
Contributor

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @philikon as a potential reviewer. Could you take a look please or cc someone with more context?

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 16, 2016
@lacker

Copy link
Copy Markdown
Contributor

Why is this the right behavior? It's not clear to me that websockets should automatically use cookies.

FWIW #10575 is a similar pull request.

@lacker

Copy link
Copy Markdown
Contributor

Chatted with some folks internally to make sure this is 1/ philosophically what we want to do and 2/ wont break FB apps and this seems good.

@facebook-github-bot shipit

@facebook-github-botfacebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Nov 7, 2016
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@lacker has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@antoinerousseau
antoinerousseau deleted the android-websocket-cookies branch November 7, 2016 18:52
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Nov 10, 2016
Summary:
This PR updates #6851 from srikanthkh, fixing coding conventions and javadoc, and adding a test plan.
Added testing functions into the WebSocketExample page of the UIExplorer, including a tiny http server to set a cookie on demand. Instructions included in the UIExplorer app.
Closesreact/react-native#9114
Differential Revision: D4140534
Pulled By: lacker
fbshipit-source-id: e020ad0c6d1d3ea09c0c3564c1795b4e1bc4517d
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.Import StartedThis pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@antoinerousseau@zxcpoiu@shirou@facebook-github-bot@lacker