Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upImplementation of HTTP Redirect Fetch step #9608
Conversation
highfive
commented
Feb 11, 2016
|
|
|
Good work! Only a few small things that need further work. Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 2 of 2 files at r4. components/net/fetch/methods.rs, line 438 [r1] (raw file): components/net/fetch/methods.rs, line 570 [r1] (raw file): let status_code = actual_response.status.unwrap();
if ((status_code == ... ||components/net/fetch/methods.rs, line 571 [r4] (raw file): Comments from the review on Reviewable.io |
|
Great work! Go ahead and squash these together! Reviewed 2 of 2 files at r5. Comments from the review on Reviewable.io |
nikkisquared
force-pushed the
nikkisquared:implement_http_redirect_fetch
branch
from
deced34
to
cf60760
|
@bors-servo: r+ |
|
|
Implementation of HTTP Redirect Fetch step I've made a first draft of a complete implementation of HTTP Redirect Fetch, most of which is just refactored out of HTTP Fetch. I've also made some minor changes in a few other steps, all collected in the second commit, based on recent changes to the Fetch Standard. Since HTTP Redirect Fetch is so new, I figured now would be a fine time to make those other changes. The biggest thing on my mind right now is how the spec says[1] "This algorithm will be used by HTML's "navigate" algorithm in addition to HTTP fetch above." This makes me think that this function, as well as HTTP Fetch, need to public, or at least have a public-facing function- since each Fetch function takes an Rc<Request>, which might be weird to require callers to supply. [1] https://fetch.spec.whatwg.org/#http-redirect-fetch <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9608) <!-- Reviewable:end -->
|
|
|
@bors-servo: retry |
Implementation of HTTP Redirect Fetch step I've made a first draft of a complete implementation of HTTP Redirect Fetch, most of which is just refactored out of HTTP Fetch. I've also made some minor changes in a few other steps, all collected in the second commit, based on recent changes to the Fetch Standard. Since HTTP Redirect Fetch is so new, I figured now would be a fine time to make those other changes. The biggest thing on my mind right now is how the spec says[1] "This algorithm will be used by HTML's "navigate" algorithm in addition to HTTP fetch above." This makes me think that this function, as well as HTTP Fetch, need to public, or at least have a public-facing function- since each Fetch function takes an Rc<Request>, which might be weird to require callers to supply. [1] https://fetch.spec.whatwg.org/#http-redirect-fetch <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9608) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
Implementation of HTTP Redirect Fetch step I've made a first draft of a complete implementation of HTTP Redirect Fetch, most of which is just refactored out of HTTP Fetch. I've also made some minor changes in a few other steps, all collected in the second commit, based on recent changes to the Fetch Standard. Since HTTP Redirect Fetch is so new, I figured now would be a fine time to make those other changes. The biggest thing on my mind right now is how the spec says[1] "This algorithm will be used by HTML's "navigate" algorithm in addition to HTTP fetch above." This makes me think that this function, as well as HTTP Fetch, need to public, or at least have a public-facing function- since each Fetch function takes an Rc<Request>, which might be weird to require callers to supply. [1] https://fetch.spec.whatwg.org/#http-redirect-fetch <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9608) <!-- Reviewable:end -->
|
|
nikkisquared commentedFeb 11, 2016
I've made a first draft of a complete implementation of HTTP Redirect Fetch, most of which is just refactored out of HTTP Fetch. I've also made some minor changes in a few other steps, all collected in the second commit, based on recent changes to the Fetch Standard. Since HTTP Redirect Fetch is so new, I figured now would be a fine time to make those other changes.
The biggest thing on my mind right now is how the spec says[1] "This algorithm will be used by HTML's "navigate" algorithm in addition to HTTP fetch above." This makes me think that this function, as well as HTTP Fetch, need to public, or at least have a public-facing function- since each Fetch function takes an Rc, which might be weird to require callers to supply.
[1] https://fetch.spec.whatwg.org/#http-redirect-fetch