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

Remove panic! from the compositor #10902

Merged

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Apr 28, 2016

Fixes #10864, and adds a check to /etc/ci/check_no_unwrap.sh.

r? @aneeshusa


This change is Reviewable

@@ -12,4 +12,4 @@ FILES=("components/compositing/compositor.rs"
"components/compositing/pipeline.rs"
"components/compositing/constellation.rs")

! grep -n "unwrap(" "${FILES[@]}"
! grep -n "unwrap(\|panic!" "${FILES[@]}"

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Apr 28, 2016

Member

Let's make this panic!( for consistency. LGTM otherwise.

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Apr 29, 2016

Author Member

Done.

@@ -1119,7 +1125,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
layer_id: LayerId) {
if let Some(point) = self.fragment_point.take() {
if !self.move_layer(pipeline_id, layer_id, Point2D::from_untyped(&point)) {
panic!("Compositor: Tried to scroll to fragment with unknown layer.");
warn!("Compositor: Tried to scroll to fragment with unknown layer.");
}

This comment has been minimized.

Copy link
@samlh

samlh Apr 28, 2016

Contributor

Return?

This comment has been minimized.

Copy link
@asajeffrey

asajeffrey Apr 29, 2016

Author Member

Done.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 29, 2016

r=me after squash

Once the buildbot config is part of this repo, I'd like to rename the check_no_unwrap.sh to check_no_panic.sh; I'll probably open that as an E-easy issue.

@asajeffrey asajeffrey force-pushed the asajeffrey:remove-explicit-panic-from-compositor branch from 3ff89a6 to 5db9287 Apr 29, 2016
@asajeffrey
Copy link
Member Author

asajeffrey commented Apr 29, 2016

@bors-servo r=aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 29, 2016

📌 Commit 5db9287 has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 30, 2016

Testing commit 5db9287 with merge f75fa52...

bors-servo added a commit that referenced this pull request Apr 30, 2016
…or, r=aneeshusa

Remove panic! from the compositor

Fixes #10864, and adds a check to `/etc/ci/check_no_unwrap.sh`.

r? @aneeshusa

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10902)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 30, 2016

@bors-servo bors-servo merged commit 5db9287 into servo:master Apr 30, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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

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