Skip to content

Various enhancements #10

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

Merged
merged 6 commits into from
Sep 16, 2014
Merged

Various enhancements #10

merged 6 commits into from
Sep 16, 2014

Conversation

kmcallister
Copy link
Contributor

Servo's Rust is too old to build this code, but I want these changes for html5ever.

We can update Servo's submodule after the next Rust upgrade (which is already needed to land html5ever).

@@ -272,6 +253,29 @@ impl fmt::Show for Atom {
}
}

impl Str for Atom {
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to leave the Str implementation until the rust team make the change in stdlib so that Str does not imply Equiv?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with this issue -- can you elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

It's in relation to this - rust-lang/rust#16738

@glennw
Copy link
Member

glennw commented Sep 4, 2014

Have you tested cross compiling to android? If not, I can test that once it's merged into servo and fix any issues if you want to do it that way.

Otherwise, looks good to me!

@kmcallister
Copy link
Contributor Author

I haven't tested cross compiling, and I don't have an easy way to do so, so I think it would be better to test once it's merged in Servo.

@kmcallister
Copy link
Contributor Author

I'll get rid of the Str impl for now.

kmcallister added a commit that referenced this pull request Sep 16, 2014
@kmcallister kmcallister merged commit ac344e1 into servo:master Sep 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants