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

Implement setters in URLUtils #7228

Merged
merged 1 commit into from Aug 30, 2015
Merged

Implement setters in URLUtils #7228

merged 1 commit into from Aug 30, 2015

Conversation

@nox
Copy link
Member

nox commented Aug 15, 2015

This fixes #6145 and takes care of most of #4250.

Review on Reviewable

@jdm
Copy link
Member

jdm commented Aug 15, 2015

@dzbarsky Would you like to review this?

@dzbarsky
Copy link
Member

dzbarsky commented Aug 15, 2015

I'm gone for the next week, but I'll do it week after next if nobody else
gets to it.

On Sat, Aug 15, 2015, 10:29 AM Josh Matthews notifications@github.com
wrote:

@dzbarsky https://github.com/dzbarsky Would you like to review this?


Reply to this email directly or view it on GitHub
#7228 (comment).

@nox
Copy link
Member Author

nox commented Aug 15, 2015

Cc'ing @SimonSapin.

@nox nox force-pushed the nox:urlutils-setters branch from 9bf29e5 to 3b02c1e Aug 17, 2015
@Ms2ger Ms2ger self-assigned this Aug 17, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 17, 2015

Reviewed 3 of 5 files at r1.
Review status: 3 of 5 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


components/script/dom/location.rs, line 88 [r1] (raw file):
This can't throw.


Comments from the review on Reviewable.io

@nox
Copy link
Member Author

nox commented Aug 17, 2015

Review status: 3 of 5 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


components/script/dom/location.rs, line 88 [r1] (raw file):
https://url.spec.whatwg.org/#dom-urlutils-href

Setter, step 2, in the URL case it can throw.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Aug 18, 2015

The latest upstream changes (presumably #7224) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox force-pushed the nox:urlutils-setters branch from 3b02c1e to 944f001 Aug 18, 2015
@nox nox removed the S-needs-rebase label Aug 18, 2015
@nox nox unassigned Ms2ger Aug 27, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Aug 27, 2015

The latest upstream changes (presumably #7401) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox force-pushed the nox:urlutils-setters branch from 944f001 to f356db9 Aug 27, 2015
@nox
Copy link
Member Author

nox commented Aug 27, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Aug 27, 2015

The latest upstream changes (presumably #7416) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox force-pushed the nox:urlutils-setters branch from e1a2cf0 to 9c4766b Aug 30, 2015
@jdm
Copy link
Member

jdm commented Aug 30, 2015

@bors-servo: r+


Reviewed 2 of 5 files at r1, 2 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r4.
Review status: 4 of 5 files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Aug 30, 2015

📌 Commit 9c4766b has been approved by jdm

@jdm
Copy link
Member

jdm commented Aug 30, 2015

Thanks for tackling this, @nox!

@bors-servo
Copy link
Contributor

bors-servo commented Aug 30, 2015

Testing commit 9c4766b with merge 347e9b6...

bors-servo pushed a commit that referenced this pull request Aug 30, 2015
Implement setters in URLUtils

This fixes #6145 and takes care of most of #4250.

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

bors-servo commented Aug 30, 2015

@bors-servo bors-servo merged commit 9c4766b into servo:master Aug 30, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@nox nox deleted the nox:urlutils-setters branch Sep 11, 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.

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