-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Update compiletest's dependencies #48798
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
This seems fine to me though I do worry somewhat about switching to serde since the diagnostics are serialized by rustc-serialize. But if tests pass, then I'm happy. @bors r+ |
📌 Commit 51f4761 has been approved by |
src/tools/compiletest/src/json.rs
Outdated
use std::str::FromStr; | ||
use std::path::Path; | ||
use runtest::ProcRes; | ||
|
||
// These structs are a subset of the ones found in | ||
// `syntax::json`. | ||
|
||
#[derive(RustcEncodable, RustcDecodable)] | ||
#[derive(Serialize, Deserialize)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does deriving Serialize
really need? I did the same thing on compiletest-rs
crate several days ago and found that it's not needed for that crate. Not sure about this in-tree version though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent point!
Drop rustc-serialize, add winapi 0.3 and Serde!
51f4761
to
db824b2
Compare
@bors: r=Mark-Simulacrum |
📌 Commit db824b2 has been approved by |
Drop rustc-serialize, add winapi 0.3 and Serde!