Skip to content

Conversation

@jbangelo
Copy link
Contributor

@jbangelo jbangelo commented Oct 31, 2019

Added a compile time feature, called serialize, which pulls in the base serde crate and derives all message types from the serde serialize and deserialize traits. In addition I updated the sbp crate to use the 2018 edition and fixed a few minor issues in the rust file templates.

@jbangelo jbangelo force-pushed the jbangelo/add-optional-serde-feature branch from d3b7c8d to 620ecd3 Compare November 1, 2019 15:59
@jbangelo
Copy link
Contributor Author

jbangelo commented Nov 1, 2019

One thing that came to mind was to add tests of the JSON serialization feature. The YAML tests contain JSON payload of the message, we could add tests that serialize a message object and make sure the resultant JSON is equivalent to the pre-generated JSON.

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.

@jbangelo up to you re. JSON tests, I think it'd add quite a bit of complexity to this PR

@jbangelo
Copy link
Contributor Author

jbangelo commented Nov 1, 2019

@silverjam Agreed that it would significantly increase the size/complexity of this PR. Was just bringing it up in case anyone thought it would be prudent to include from the get go. I'll consider it a "nice to have" for the future.

Copy link
Contributor

@benjaminaltieri benjaminaltieri left a comment

Choose a reason for hiding this comment

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

look good to me. is there an example or test that showcases this new feature?

@jbangelo
Copy link
Contributor Author

jbangelo commented Nov 1, 2019

Merging as is. It looks like travis somehow missed the update. There's one failure in the travis build, but it's the pythong/haskell speed comparison which occasionally fails anyways.

@jbangelo jbangelo merged commit 3e5fc99 into master Nov 1, 2019
@jbangelo jbangelo deleted the jbangelo/add-optional-serde-feature branch November 1, 2019 23:07
@jbangelo
Copy link
Contributor Author

jbangelo commented Nov 1, 2019

@benjaminaltieri There isn't an example at the moment. I'll have to resurrect my sbp2json in rust.

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