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.

Sign up for GitHub

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

Jump to bottom

Supress panics for ProfilerChan sends. #14165

Merged
merged 1 commit into from Nov 15, 2016
Merged

Supress panics for ProfilerChan sends. #14165

merged 1 commit into from Nov 15, 2016

Conversation

jdm
Copy link
Member

@jdm jdm commented Nov 10, 2016 •
edited by larsbergstrom

This should address #14161.

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @fitzgen: components/profile_traits/mem.rs, components/profile_traits/mem.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 10, 2016
@nox
Copy link
Contributor

nox commented Nov 10, 2016

@jdm In other places we do a warn! on such errors, should it be done here too?

Cc @asajeffrey.

@asajeffrey
Copy link
Member

Yes, including the warn! might be useful. In production use, this means the warning will end up in the crash log if there's a later panic.

@jdm
Copy link
Member Author

jdm commented Nov 12, 2016

This will fix #14161 as well.

@jdm
Copy link
Member Author

jdm commented Nov 14, 2016

r? @nox

@highfive highfive assigned nox and unassigned pcwalton Nov 14, 2016
@nox
Copy link
Contributor

nox commented Nov 14, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 25a237c has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 14, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit 25a237c with merge 65dd6d4...

bors-servo pushed a commit that referenced this pull request Nov 14, 2016
Auto merge of #14165 - servo:jdm-patch-1, r=nox
65dd6d4 
Supress panics for ProfilerChan sends.

This should address #14161.

<!-- 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/14165)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 14, 2016
@jdm
Copy link
Member Author

jdm commented Nov 14, 2016

@bors-servo: retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for arm32, arm64, linux-dev, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-css...

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Nov 15, 2016
@jdm
Copy link
Member Author

jdm commented Nov 15, 2016

@bors-servo: retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for arm32, arm64, linux-dev, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-css...

@bors-servo
Copy link
Contributor

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

@bors-servo bors-servo merged commit 25a237c into master Nov 15, 2016
@timvandermeij timvandermeij mentioned this pull request Nov 15, 2016
Closed
@SimonSapin SimonSapin deleted the jdm-patch-1 branch November 21, 2016 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jdm @highfive @nox @asajeffrey @bors-servo @pcwalton