Skip to content

/ servo

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

Kill some more plugins #15545

Merged
merged 3 commits into from Feb 15, 2017
Merged

Kill some more plugins #15545

merged 3 commits into from Feb 15, 2017

Conversation

@nox
Copy link
Member

nox commented Feb 14, 2017 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Feb 14, 2017

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/lib.rs, components/script/dom/eventtarget.rs, components/script/timers.rs, components/script/Cargo.toml, components/script/dom/range.rs, components/script/dom/bindings/iterable.rs
  • @KiChjang: components/script/lib.rs, components/script/dom/eventtarget.rs, components/script/timers.rs, components/script/Cargo.toml, components/script/dom/range.rs, components/script/dom/bindings/iterable.rs
@SimonSapin
Copy link
Member

SimonSapin commented Feb 14, 2017

r=me with or without the renames, as you feel like.

Reviewed 4 of 4 files at r1, 16 of 16 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

components/privatize_derive/Cargo.toml, line 2 at r2 (raw file):

[package]
name = "privatize_derive"

Super nit: call this deny_public_fields to align with the derive name?

components/privatize_derive/lib.rs, line 9 at r2 (raw file):

extern crate synstructure;

#[proc_macro_derive(deny_public_fields)]

Super nit: DenyPublicFields?

Comments from Reviewable
@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 14, 2017 

The command "./mach test-compiletest" exited with 101.
@Manishearth
Manishearth approved these changes Feb 14, 2017
View changes
Copy link
Member

Manishearth left a comment

r=me
components/script/dom/bindings/iterable.rs
pub struct IterableIterator<T: DomObject + JSTraceable + Iterable> {
reflector: Reflector,
iterable: JS<T>,
type_: IteratorType,
index: Cell<u32>,
}

impl<T: DomObject + JSTraceable + Iterable> DomObject for IterableIterator<T> {

This comment has been minimized.

Sign in to view
@Manishearth

Manishearth Feb 14, 2017

Member

Why did this need to be removed?

This comment has been minimized.

Sign in to view
@SimonSapin

SimonSapin Feb 14, 2017

Member

#[dom_struct] adds #[derive(DomObject)], which provides this impl.
@jdm jdm added the S-fails-travis label Feb 14, 2017
@nox nox force-pushed the nox:plugin branch from bd516d4 to 9553522 Feb 15, 2017
@nox
Kill transmute-type-lint
19c645f
@nox nox force-pushed the nox:plugin branch from 9553522 to b3f6fe8 Feb 15, 2017
nox added 2 commits Feb 14, 2017
@nox
Change #[privatize] into #[derive(DenyPublicFields)]
8bcf36b
@nox
Use #[dom_struct] on IterableIterator<T> (fixes #12811)
Loading status checks…
be000d3
@nox nox force-pushed the nox:plugin branch from dbdbd83 to be000d3 Feb 15, 2017
@nox
Copy link
Member Author

nox commented Feb 15, 2017

@bors-servo r=SimonSapin
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2017

📌 Commit be000d3 has been approved by SimonSapin
@highfive highfive assigned SimonSapin and unassigned pcwalton Feb 15, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2017

⌛ Testing commit be000d3 with merge 9702d69...
bors-servo added a commit that referenced this pull request Feb 15, 2017
@bors-servo
Auto merge of #15545 - nox:plugin, r=SimonSapin
Loading status checks…
9702d69 
Kill some more plugins

<!-- 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/15545)
<!-- Reviewable:end -->
@bors-servo bors-servo mentioned this pull request Feb 15, 2017
Merged
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: SimonSapin
Pushing 9702d69 to master...
@bors-servo bors-servo merged commit be000d3 into servo:master Feb 15, 2017
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
This was referenced Feb 15, 2017
Merged
Merged
@nox nox deleted the nox:plugin branch Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonSapin SimonSapin

@Manishearth Manishearth

Assignees

@SimonSapin SimonSapin

Labels
S-fails-travis
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@nox @highfive @SimonSapin @Ms2ger @bors-servo @Manishearth @jdm @pcwalton
You can’t perform that action at this time.