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

Replaced ZERO_POINT with Point2D::zero() #8796

Merged
merged 1 commit into from Dec 4, 2015

Conversation

@pointlessone
Copy link
Contributor

pointlessone commented Dec 3, 2015

This is a proposed in #8792 clean up.

Fixes #8792.

Review on Reviewable

@highfive
Copy link

highfive commented Dec 3, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @mbrubeck (or someone else) soon.

@highfive
Copy link

highfive commented Dec 3, 2015

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
@SimonSapin
Copy link
Member

SimonSapin commented Dec 3, 2015

@bors-servo r+

Looks good, thanks! By the way, related changes like #8795 and this can be made with multiple commits in the same PR. You can also add commits to a PR after opening it. (No need to change these now, though.)

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

📌 Commit f15ce9f has been approved by SimonSapin

@pointlessone
Copy link
Contributor Author

pointlessone commented Dec 3, 2015

@SimonSapin Thanks for the tip.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

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

@pointlessone pointlessone force-pushed the pointlessone:zero-point branch from f15ce9f to 012b3b6 Dec 3, 2015
@pointlessone pointlessone force-pushed the pointlessone:zero-point branch from 012b3b6 to ee746e2 Dec 3, 2015
@SimonSapin
Copy link
Member

SimonSapin commented Dec 3, 2015

Not sure why bors says it’s unmergeable, Github seems fine with it.

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

📌 Commit ee746e2 has been approved by SimonSapin

@SimonSapin
Copy link
Member

SimonSapin commented Dec 3, 2015

Oh, you’ve already rebased :)

There’s no email notification when new commits are pushed in a PR, so feel free to add a comment when you do.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

Testing commit ee746e2 with merge d40051f...

bors-servo added a commit that referenced this pull request Dec 3, 2015
Replaced ZERO_POINT with Point2D::zero()

This is a proposed in #8792 clean up.

Fixes #8792.

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

bors-servo commented Dec 3, 2015

💔 Test failed - linux-rel

@KiChjang
Copy link
Member

KiChjang commented Dec 3, 2015


Ran 3726 tests finished in 198.0 seconds.
  • 3725 ran as expected. 708 tests skipped.
  • 1 tests timed out unexpectedly

Tests with unexpected results:
  ▶ TIMEOUT [expected FAIL] /html/dom/elements/global-attributes/dir_auto-N-EN.html
  │ 
  │ was ready to save, resetting ready_to_save_state
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ was ready to save, resetting ready_to_save_state
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ thread 'Constellation' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 104, message: "Connection reset by peer" } }', ../src/libcore/result.rs:741
  │ stack backtrace:
  │    1:     0x7feb8058e3a0 - sys::backtrace::tracing::imp::write::h064b2475ee0d772cxXt
  │    2:     0x7feb805912d5 - panicking::log_panic::_<closure>::closure.40992
  │    3:     0x7feb80590d50 - panicking::log_panic::h29f8d24759545d7862x
  │    4:     0x7feb8057a003 - sys_common::unwind::begin_unwind_inner::hdadcc96a1ecb24974Ps
  │    5:     0x7feb8057a6b8 - sys_common::unwind::begin_unwind_fmt::h9442bd88fc02b35caPs
  │    6:     0x7feb8058da01 - rust_begin_unwind
  │    7:     0x7feb805be46f - panicking::panic_fmt::h2f3428e3725b99d6VFK
  │    8:     0x7feb7ef6dc65 - constellation::_<impl>::handle_is_ready_to_save_image::handle_is_ready_to_save_image::h7464060649015599629
  │    9:     0x7feb7eefb4e5 - constellation::_<impl>::handle_request::handle_request::h2512767965772658699
  │   10:     0x7feb7eee5f47 - sys_common::unwind::try::try_fn::try_fn::h18033304662745029905
  │   11:     0x7feb8058d848 - __rust_try
  │   12:     0x7feb80589c1b - sys_common::unwind::try::inner_try::hb6fbb19d6020cb51CMs
  │   13:     0x7feb7eee81ba - boxed::_<impl>::call_box::call_box::h3977225798282015024
  │   14:     0x7feb8058fc33 - sys::thread::_<impl>::new::thread_start::h51b5333730e2e1a3I5w
  │   15:     0x7feb7c53a181 - start_thread
  │   16:     0x7feb7cd5447c - __clone
  └   17:                0x0 - <unknown>
@KiChjang
Copy link
Member

KiChjang commented Dec 3, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

Previous build results for android, gonk, linux-dev, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...

@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

💔 Test failed - linux-rel

@KiChjang
Copy link
Member

KiChjang commented Dec 3, 2015

Ran 3726 tests finished in 198.0 seconds.
  • 3725 ran as expected. 708 tests skipped.
  • 1 tests failed unexpectedly

Tests with unexpected results:
  ▶ FAIL [expected PASS] /_mozilla/css/iframe/positioning_margin.html
  └   → /_mozilla/css/iframe/positioning_margin.html d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c
/_mozilla/css/iframe/positioning_margin_ref.html 3a4684450cbc08c6e80ab78eb761e71881c74ab1
Testing d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c == 3a4684450cbc08c6e80ab78eb761e71881c74ab1
@jdm
Copy link
Member

jdm commented Dec 4, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Dec 4, 2015

Testing commit ee746e2 with merge 2dbc314...

bors-servo added a commit that referenced this pull request Dec 4, 2015
Replaced ZERO_POINT with Point2D::zero()

This is a proposed in #8792 clean up.

Fixes #8792.

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

bors-servo commented Dec 4, 2015

@bors-servo bors-servo merged commit ee746e2 into servo:master Dec 4, 2015
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable Review in progress: 0 of 7 files reviewed, all discussions resolved
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@pointlessone
Copy link
Contributor Author

pointlessone commented Dec 4, 2015

There’s no email notification when new commits are pushed in a PR, so feel free to add a comment when you do.

Will do.

Is this OK for tests to pass only after multiple retries?

@pointlessone pointlessone deleted the pointlessone:zero-point branch Dec 4, 2015
@SimonSapin
Copy link
Member

SimonSapin commented Dec 4, 2015

Some of our tests are known to have "intermittent" results. This is a bug, but it’s sometimes hard to track down. When it gets bad we sometimes disable some tests. To avoid bloking everything, it’s acceptable to retry when linking to an issue like #8815 about the relevant test, opening one if necessary.

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.