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

compositing: Make the constellation messages serializable. #6593

Merged
merged 1 commit into from Jul 18, 2015

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Jul 11, 2015

Same idea as before. This depends on:

r? @jdm

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jul 11, 2015

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

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.

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2015

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

@jdm
Copy link
Member

jdm commented Jul 14, 2015

-S-awaiting-review +S-needs-code-changes


Reviewed 33 of 33 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


components/msg/Cargo.toml, line 37 [r1] (raw file):
Let's go back to the cargo version.


components/msg/Cargo.toml, line 41 [r1] (raw file):
Let's go back to the cargo version.


Comments from the review on Reviewable.io

@pcwalton pcwalton force-pushed the pcwalton:constellation-ipc branch from df90fef to 383b917 Jul 14, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 14, 2015

Fixed. r? @jdm

Note that this is currently blocked on serde: serde-rs/serde#102

@jdm
Copy link
Member

jdm commented Jul 14, 2015

-S-awaiting-review +S-blocked-on-external


Reviewed 21 of 21 files at r2.
Review status: :shipit: all 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 Jul 14, 2015

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

@pcwalton pcwalton force-pushed the pcwalton:constellation-ipc branch from 383b917 to 1f94cdc Jul 16, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 16, 2015

Rebased. Still blocked on @erickt reviewing serde-rs/serde#102

@bors-servo
Copy link
Contributor

bors-servo commented Jul 16, 2015

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

@erickt
Copy link

erickt commented Jul 16, 2015

@pcwalton: I merged in your PR. Do you need a release cut for this as well?

@jdm jdm removed the S-awaiting-review label Jul 17, 2015
@jdm
Copy link
Member

jdm commented Jul 17, 2015

-S-awaiting-review -S-blocked-on-external +S-needs-rebase


Reviewed 21 of 21 files at r2, 19 of 19 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from the review on Reviewable.io

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 18, 2015

@bors-servo: r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

📌 Commit 59d9c4f has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

Testing commit 59d9c4f with merge 7ad6dd8...

bors-servo pushed a commit that referenced this pull request Jul 18, 2015
compositing: Make the constellation messages serializable.

Same idea as before. This depends on:

* servo/euclid#94
* hyperium/hyper#603

r? @jdm

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

bors-servo commented Jul 18, 2015

💔 Test failed - mac3

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 18, 2015

@bors-servo: retry

Network failure

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

Previous build results for android, gonk are reusable. Rebuilding only linux1, linux2, linux3, mac1, mac2, mac3...

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

💔 Test failed - linux2

@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 18, 2015

OK, that's odd. A URL parsing test case no longer fails. Perhaps the fact that we serialize URLs via strings between the constellation and the rest of Servo has caused this?

@pcwalton pcwalton force-pushed the pcwalton:constellation-ipc branch from 59d9c4f to b6485a9 Jul 18, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 18, 2015

@bors-servo: r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

📌 Commit b6485a9 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 18, 2015

Testing commit b6485a9 with merge 2f4bdc6...

bors-servo pushed a commit that referenced this pull request Jul 18, 2015
compositing: Make the constellation messages serializable.

Same idea as before. This depends on:

* servo/euclid#94
* hyperium/hyper#603

r? @jdm

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

bors-servo commented Jul 18, 2015

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

@jdm

This comment has been minimized.

Copy link
Member

jdm commented on b6485a9 Jul 22, 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.

None yet

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