Drop rustc-serialize dependency

[ordian]

- ~~~encodable* types were renamed to serializable* for consistency with serde~~~
- error messages were tweaked a little bit

~~~TODO (migrate in dependencies):~~~
- [x] ~~~update clippy~~~
- [ ] [mime-types](https://github.com/conduit-rust/mime-types) ([#8](https://github.com/conduit-rust/mime-types/pull/8))
- [ ] [conduit-cookie](https://github.com/conduit-rust/conduit-cookie) ([#3](https://github.com/conduit-rust/conduit-cookie/pull/3), [#4](https://github.com/conduit-rust/conduit-cookie/pull/4))
- [ ] [conduit-json-parser](https://github.com/conduit-rust/conduit-json-parser) ([#3](https://github.com/conduit-rust/conduit-json-parser/pull/3))  

Closes #583.

[ordian]

So this is ready to merge.

It doesn't drop rustc-serialize in transitive dependencies yet, but this can be done in a separate PR.
To do that we need a version bump from mime-types, conduit-cookie, conduit-json-parser, also
conduit-static needs to be updated on a new mime-types and bump its version as well (or we can switch from conduit to other web application interface).

cc @carols10cents what do you think?

[sgrif]

This seems unrelated to the other changes in this PR

[ordian]

This is related to updating clippy. Old clippy (0.0.118) depended on rustc-serialize (via toml 0.2) and the new one (0.0.141) won't build with nightly-2017-03-04.

[sgrif]

Is the rename of encodable to serializable really necessary? It seems like it just adds a ton of churn for very little gain. After switching to Serde I suspect we can just eliminate a lot of these anyway.

[ordian]

Is the rename of encodable to serializable really necessary? It seems like it just adds a ton of churn for very little gain. After switching to Serde I suspect we can just eliminate a lot of these anyway.

Good point. I reverted this change.

I would prefer to eliminate encodable structs in a separate pr, because currently it is blocked on postgres release with chrono 0.4.

[sgrif]

Yes, I agree that the elimination should not be part of this PR>

[sgrif]

The semver crate has a serde feature. Should we just use that instead?

[ordian]

I tried to at first, but failed. Because of conduit, we must use semver 0.5, which doesn't have serde feature.

[sgrif]

I'm a little worried about whether this changes our response structure in subtle ways that will break the front-end and won't be caught by our tests, but overall this seems good.

[ordian]

I'm a little worried about whether this changes our response structure in subtle ways that will break the front-end and won't be caught by our tests

This PR doesn't change json structure, only the encoder/decoder is changed. Assuming serde_json is correct, it won't break the front-end.

[carols10cents]

Nice!! Thank you for this!!! Love how it gets rid of things like the fixup function and just makes it a serde rename 👍 💯

also
conduit-static needs to be updated on a new mime-types and bump its version as well (or we can switch from conduit to other web application interface).

Bumping the versions of anything out of date sounds great-- I'd rather do that than switch to another web app framework right now. I'm not opposed to switching from conduit to something else, it'd just be a bigger project, a larger change that can't easily be made incrementally, and that kind of thing makes me nervous :) If switching frameworks makes the code easier to understand and possibly more familiar for contributors, I'm up for being convinced that it's worth the tradeoff though :)

The error messages printed through cargo when a publish gets rejected are a little bit uglier. Before this change:

error: api errors: invalid upload request: ApplicationError("invalid crate name specified: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa. Valid crate names must start with a letter; contain only letters, numbers, hyphens, or underscores; and have 64 or fewer characters.")

after:

error: api errors: invalid upload request: ErrorImpl { code: Message("invalid value: string \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters"), line: 1, column: 94 }

Just that extra ErrorImpl { code: Message( where it only said ApplicationError before. I'm going to merge this anyway since people publishing crates should be fine reading Rust errors, just something to think about and maybe fix if folks complain. Not sure if the fix to this would be in crates.io or in cargo...

[ordian]

I'm not opposed to switching from conduit to something else, it'd just be a bigger project, a larger change that can't easily be made incrementally, and that kind of thing makes me nervous :)

I understand the concern.

If switching frameworks makes the code easier to understand and possibly more familiar for contributors, I'm up for being convinced that it's worth the tradeoff though :)

It probably would, since conduit is not maintained. But it's better to wait for a framework with a mature http2 and async support (hyper is anync, but not a framework, iron is not async yet).

Just that extra ErrorImpl { code: Message( where it only said ApplicationError before.

Does this PR fix the issue?