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 upImplement connectStart in PerformanceResourceTiming #23323
Conversation
highfive
commented
May 5, 2019
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon. |
highfive
commented
May 5, 2019
|
Heads up! This PR modifies the following files:
|
|
@bors-servo r+ |
|
|
Implement connectStart in PerformanceResourceTiming <!-- Please describe your changes on the following line: --> - Added connectStart where needed (```ResourceFetchTiming```, ```ResourceAttribute```) in ```components/net_traits/lib.rs ``` - Before calling ```client.request``` in ```obtain_response```, we now add the ```connectStart``` attribute (```components/net/http_loader.rs```) - Updated tests to reflect those changes --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [X] These changes fix #21261 (at least partially) <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23323) <!-- Reviewable:end -->
|
|
|
@bors-servo r+ |
|
|
|
|
|
@bors-servo r- |
|
It looks like my previous comments have not been precise enough. Your original changes were fine, with the exception of one particular test result which needed to be removed. |
|
@bors-servo r+ |
|
|
Implement connectStart in PerformanceResourceTiming <!-- Please describe your changes on the following line: --> - Added connectStart where needed (```ResourceFetchTiming```, ```ResourceAttribute```) in ```components/net_traits/lib.rs ``` - Before calling ```client.request``` in ```obtain_response```, we now add the ```connectStart``` attribute (```components/net/http_loader.rs```) - Updated tests to reflect those changes --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [X] These changes fix #21261 (at least partially) <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23323) <!-- Reviewable:end -->
|
|
|
Is #21261 finished or is there still more to do for that? |
nehalem501 commentedMay 5, 2019
•
edited
ResourceFetchTiming,ResourceAttribute) incomponents/net_traits/lib.rsclient.requestinobtain_response, we now add theconnectStartattribute (components/net/http_loader.rs)./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is