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

Throw a SyntaxError if the WebSocket URL can't be parsed. Fix #6061 #6127

Merged
merged 5 commits into from May 20, 2015

Conversation

@Nashenas88
Copy link

Nashenas88 commented May 18, 2015

Fix #6061.

Review on Reviewable

@highfive
Copy link

highfive commented May 18, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @SimonSapin (or someone else) soon.

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 18, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5043

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@SimonSapin SimonSapin self-assigned this May 19, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented May 19, 2015

Reviewed on critic (though I need to have another look at parse_web_socket_url).

@SimonSapin
Copy link
Member

SimonSapin commented May 20, 2015

Looks good! Please squash the commits, and this will be good to go. I’ve also edited your PR message to include "Fix #6061" (rather than just the number) so that GitHub will close the issue automatically when this is merged.

@Nashenas88 Nashenas88 force-pushed the Nashenas88:websockets-invalid-urls branch from 836c62f to d7387ed May 20, 2015
@Nashenas88 Nashenas88 changed the title Throw a SyntaxError if the WebSocket URL can't be parsed. #6061 Throw a SyntaxError if the WebSocket URL can't be parsed. Fix #6061 May 20, 2015
@Nashenas88
Copy link
Author

Nashenas88 commented May 20, 2015

Should the S-needs-squash tag get cleared automatically or did I make a mistake?

@SimonSapin
Copy link
Member

SimonSapin commented May 20, 2015

I don’t think our automation is that good yet. But what did you change? I still see 7 commits.

@Nashenas88 Nashenas88 force-pushed the Nashenas88:websockets-invalid-urls branch from d7387ed to 51ae733 May 20, 2015
@Manishearth
Copy link
Member

Manishearth commented May 20, 2015

@bors-servo: r=Ms2ger

@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

📌 Commit 51ae733 has been approved by Ms2ger

@Nashenas88
Copy link
Author

Nashenas88 commented May 20, 2015

I was doing the rebasing incorrectly. I was passing --autosquash without --interactive.

@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

Testing commit 51ae733 with merge d869a3c...

bors-servo pushed a commit that referenced this pull request May 20, 2015
Fix #6061.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6127)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2

@bors-servo bors-servo merged commit 51ae733 into servo:master May 20, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Nashenas88 Nashenas88 deleted the Nashenas88:websockets-invalid-urls branch May 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

9 participants
You can’t perform that action at this time.