Skip to content
This repository was archived by the owner on Nov 12, 2022. It is now read-only.

Added RootKind and GCMethods implementation for PropertyDescriptor #414

Merged
merged 1 commit into from
Apr 6, 2018

Conversation

taki-jaro
Copy link
Contributor

@taki-jaro taki-jaro commented Mar 27, 2018

It is needed for servo issue 15012.


This change is Reviewable

src/rust.rs Outdated
@@ -304,6 +304,11 @@ impl RootKind for Value {
fn rootKind() -> jsapi::RootKind { jsapi::RootKind::Value }
}

impl RootKind for PropertyDescriptor {
#[inline(always)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: indentation.

impl GCMethods for PropertyDescriptor {
unsafe fn initial() -> PropertyDescriptor { PropertyDescriptor::default() }
unsafe fn post_barrier(_ : *mut PropertyDescriptor, _ : PropertyDescriptor, _ :PropertyDescriptor) {}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add a newline after this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Cargo.toml Outdated
@@ -2,7 +2,7 @@
name = "mozjs"
description = "Rust bindings to the Mozilla SpiderMonkey JavaScript engine."
repository = "https://github.com/servo/rust-mozjs"
version = "0.5.0"
version = "0.6.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be 0.5.1.

@taki-jaro taki-jaro force-pushed the property_descriptor branch 3 times, most recently from fb073ab to 18ea0f8 Compare March 27, 2018 20:17
@jdm
Copy link
Member

jdm commented Mar 27, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 18ea0f8 has been approved by jdm

@bors-servo
Copy link
Contributor

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

@bors-servo
Copy link
Contributor

🔒 Merge conflict

@taki-jaro
Copy link
Contributor Author

Why this stiil isn't merged 2 hours after passing tests and approval?

@jdm
Copy link
Member

jdm commented Mar 28, 2018

I didn't think there was any particular rush, since there's still debugging happening for the test failures, right? I've been too hasty merging other PRs like the handle one while the Servo PR isn't ready yet.

@taki-jaro
Copy link
Contributor Author

No there is no particular rush.I was just surprised and thought there is a problem on my side.

@KiChjang
Copy link
Contributor

@taki-jaro Could you rebase your commits against the master branch instead of merging? Once you've done that, you can instruct bors-servo to r=jdm.

@bors-servo delegate+

@bors-servo
Copy link
Contributor

✌️ @taki-jaro can now approve this pull request

@taki-jaro taki-jaro force-pushed the property_descriptor branch from 5fb61e7 to 3e5024a Compare April 5, 2018 16:16
@taki-jaro
Copy link
Contributor Author

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

📌 Commit 3e5024a has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 3e5024a with merge 22bc648...

bors-servo pushed a commit that referenced this pull request Apr 5, 2018
Added RootKind and GCMethods implementation for PropertyDescriptor

It is needed for [servo issue 15012](servo/servo#15012).

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

💔 Test failed - status-travis

@KiChjang
Copy link
Contributor

KiChjang commented Apr 5, 2018

@taki-jaro No need for a new commit; you can just @bors-servo retry in order to re-trigger testing. Once you remove that commit, please @bors-servo r=jdm again.

@bors-servo
Copy link
Contributor

📌 Commit 1bff572 has been approved by `jdm``

@bors-servo
Copy link
Contributor

⌛ Testing commit 1bff572 with merge 96e1c30...

bors-servo pushed a commit that referenced this pull request Apr 5, 2018
Added RootKind and GCMethods implementation for PropertyDescriptor

It is needed for [servo issue 15012](servo/servo#15012).

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

KiChjang commented Apr 5, 2018

@bors-servo r-

@bors-servo
Copy link
Contributor

☀️ Test successful - status-appveyor, status-travis
State: approved= try=False

@taki-jaro taki-jaro force-pushed the property_descriptor branch from 1bff572 to 3e5024a Compare April 6, 2018 11:06
@taki-jaro
Copy link
Contributor Author

@KiChjang Ok, thanks

@taki-jaro
Copy link
Contributor Author

@bors-servo retry

@taki-jaro
Copy link
Contributor Author

@bors-servo retry

@taki-jaro
Copy link
Contributor Author

@KiChjang It doesn't seem to work I still have failed test from 22 hours ago, the 2 retry command I gave didin't restart the test

@KiChjang
Copy link
Contributor

KiChjang commented Apr 6, 2018

@taki-jaro When you push a new commit, the r+ state in homu is reset, which forces you to do another r+. So if I do @bors-servo r=jdm right now, it would Just Work.

@bors-servo
Copy link
Contributor

📌 Commit 3e5024a has been approved by jdm

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 3e5024a has been approved by KiChjang

@bors-servo
Copy link
Contributor

⌛ Testing commit 3e5024a with merge d027dbb...

bors-servo pushed a commit that referenced this pull request Apr 6, 2018
Added RootKind and GCMethods implementation for PropertyDescriptor

It is needed for [servo issue 15012](servo/servo#15012).

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

☀️ Test successful - status-appveyor, status-travis
Approved by: KiChjang
Pushing d027dbb to master...

@bors-servo bors-servo merged commit 3e5024a into servo:master Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants