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

Add accountable-refcell as optional build time feature #25451

Merged
merged 1 commit into from Jan 8, 2020

Conversation

@kunalmohan
Copy link
Collaborator

kunalmohan commented Jan 7, 2020

It can be used with the format ./mach build --features refcell_backtrace.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #19308 (GitHub issue number if applicable)
  • These changes do not require tests because it adds a optional build-time feature only.
@highfive
Copy link

highfive commented Jan 7, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/request.rs, components/script/dom/htmlcanvaselement.rs, components/script/dom/mediastream.rs, components/script/dom/document.rs, components/script/dom/element.rs and 16 more
  • @cbrewster: components/constellation/Cargo.toml
  • @KiChjang: components/script/dom/request.rs, components/script/dom/htmlcanvaselement.rs, components/script/dom/mediastream.rs, components/script/dom/document.rs, components/script/dom/element.rs and 15 more
@highfive
Copy link

highfive commented Jan 7, 2020

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@kunalmohan
Copy link
Collaborator Author

kunalmohan commented Jan 7, 2020

r?@jdm

@highfive highfive assigned jdm and unassigned SimonSapin Jan 7, 2020
Copy link
Member

jdm left a comment

We should add a CI job so this doesn't break, too - you can replace https://github.com/servo/servo/blob/master/etc/taskcluster/decision_task.py#L209 with refcell_backtrace.

components/constellation/Cargo.toml Outdated Show resolved Hide resolved
components/script/body.rs Outdated Show resolved Hide resolved
components/script/dom/response.rs Show resolved Hide resolved
ports/glutin/Cargo.toml Outdated Show resolved Hide resolved
ports/glutin/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/api/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/api/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/capi/Cargo.toml Outdated Show resolved Hide resolved
ports/libsimpleservo/capi/Cargo.toml Outdated Show resolved Hide resolved
@jdm
jdm approved these changes Jan 7, 2020
Copy link
Member

jdm left a comment

Looks good! Please squash and I'll merge!

@kunalmohan
Copy link
Collaborator Author

kunalmohan commented Jan 7, 2020

Done. Thanks for guiding me through this; learnt a lot while working on this issue. 😄

@jdm
Copy link
Member

jdm commented Jan 7, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2020

📌 Commit 3b37d8b has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2020

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

@jdm
Copy link
Member

jdm commented Jan 8, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

📌 Commit 02c1612 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

Testing commit 02c1612 with merge 133e514...

bors-servo added a commit that referenced this pull request Jan 8, 2020
Add accountable-refcell as optional build time feature

<!-- Please describe your changes on the following line: -->
It can be used with the format `./mach build --features refcell_backtrace`.

---
<!-- 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 #19308  (GitHub issue number if applicable)

<!-- Either: -->
- [X] These changes do not require tests because it adds a optional build-time feature only.

<!-- 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 8, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

Testing commit 02c1612 with merge f70a228...

bors-servo added a commit that referenced this pull request Jan 8, 2020
Add accountable-refcell as optional build time feature

<!-- Please describe your changes on the following line: -->
It can be used with the format `./mach build --features refcell_backtrace`.

---
<!-- 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 #19308  (GitHub issue number if applicable)

<!-- Either: -->
- [X] These changes do not require tests because it adds a optional build-time feature only.

<!-- 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 8, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

Testing commit 02c1612 with merge 0231ff0...

bors-servo added a commit that referenced this pull request Jan 8, 2020
Add accountable-refcell as optional build time feature

<!-- Please describe your changes on the following line: -->
It can be used with the format `./mach build --features refcell_backtrace`.

---
<!-- 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 #19308  (GitHub issue number if applicable)

<!-- Either: -->
- [X] These changes do not require tests because it adds a optional build-time feature only.

<!-- 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 8, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

Testing commit 02c1612 with merge 821de45...

bors-servo added a commit that referenced this pull request Jan 8, 2020
Add accountable-refcell as optional build time feature

<!-- Please describe your changes on the following line: -->
It can be used with the format `./mach build --features refcell_backtrace`.

---
<!-- 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 #19308  (GitHub issue number if applicable)

<!-- Either: -->
- [X] These changes do not require tests because it adds a optional build-time feature only.

<!-- 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. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 821de45 to master...

@bors-servo bors-servo merged commit 02c1612 into servo:master Jan 8, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@kunalmohan kunalmohan deleted the kunalmohan:19308-AccountableRefCell branch Jan 8, 2020
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.

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