Skip to content

Conversation

@jbangelo
Copy link
Contributor

@jbangelo jbangelo commented Nov 7, 2019

Just a few odds and ends that I felt should be cleaned up. Already split up into logical commits if you prefer.

  1. Added a few "keywords" to the Cargo.toml
  2. Fixed a crash in the parser caused when the buffer ran out of data and didn't get refilled
  3. Added a very bare bones description of the crate for the doc generation
  4. Added a more formally defined Result and Error type for the crate to use

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

LGTM in general, added a few comment though

@benjaminaltieri
Copy link
Contributor

I got this warning when compiling using make test-rust, with rustc==1.3.8:

warning: trait objects without an explicit `dyn` are deprecated
warning: trait objects without an explicit `dyn` are deprecated
   --> src/parser/mod.rs:134:30
    |
134 | pub fn read_string(buf: &mut Read) -> Result<String, ::Error> {
    |                              ^^^^ help: use `dyn`: `dyn Read`
    |
    = note: `#[warn(bare_trait_objects)]` on by default

   --> src/parser/mod.rs:134:30
    |
134 | pub fn read_string(buf: &mut Read) -> Result<String, ::Error> {
    |                              ^^^^ help: use `dyn`: `dyn Read`
    |
    = note: `#[warn(bare_trait_objects)]` on by default

warning: trait objects without an explicit `dyn` are deprecated
   --> src/parser/mod.rs:140:36
    |
140 | pub fn read_string_limit(buf: &mut Read, n: u64) -> Result<String, ::Error> {
    |                                    ^^^^ help: use `dyn`: `dyn Read`

warning: trait objects without an explicit `dyn` are deprecated
   --> src/parser/mod.rs:140:36
    |
140 | pub fn read_string_limit(buf: &mut Read, n: u64) -> Result<String, ::Error> {
    |                                    ^^^^ help: use `dyn`: `dyn Read`

Worth addressing?

@jbangelo
Copy link
Contributor Author

jbangelo commented Nov 8, 2019

@benjaminaltieri I think you're on the wrong branch? The dyn has already been added there.

@jbangelo jbangelo merged commit 8958389 into master Nov 8, 2019
@jbangelo jbangelo deleted the jbangelo/rust-misc-cleanup branch November 8, 2019 03:44
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.

4 participants