Skip to content
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

Replace serde_yaml with serde_json internally #518

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

rezvaneh
Copy link
Collaborator

@rezvaneh rezvaneh commented Apr 19, 2022

We replaced serde_yaml with serde_json to simplify writing tests and conversion to protobuf

Which issue(s) this PR fixes:
#507

@rezvaneh rezvaneh changed the title replace serde_yaml replace serde_yaml with serde_json Apr 19, 2022
@rezvaneh rezvaneh changed the title replace serde_yaml with serde_json replace serde_yaml with serde_json internally Apr 19, 2022
@rezvaneh rezvaneh changed the title replace serde_yaml with serde_json internally Replace serde_yaml with serde_json internally Apr 19, 2022
Copy link
Contributor

@bombsimon bombsimon left a comment

Choose a reason for hiding this comment

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

This surely looks way more convenient, LGTM!

@rezvaneh rezvaneh marked this pull request as ready for review April 19, 2022 09:49
Copy link
Collaborator

@XAMPPRocky XAMPPRocky left a comment

Choose a reason for hiding this comment

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

LGTM! Just need to run cargo fmt and fix the cargo clippy lints, and it should be good to go.

@rezvaneh
Copy link
Collaborator Author

LGTM! Just need to run cargo fmt and fix the cargo clippy lints, and it should be good to go.

I had ran them before, can you see what error says?

@XAMPPRocky
Copy link
Collaborator

Step #4 - "test-quilkin": error: casting integer literal to `i64` is unnecessary
Step #4 - "test-quilkin":    --> src/filters/capture.rs:144:25
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin": 144 |                 "size": 3 as i64,
Step #4 - "test-quilkin":     |                         ^^^^^^^^ help: try: `3_i64`
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin":     = note: `-D clippy::unnecessary-cast` implied by `-D warnings`
Step #4 - "test-quilkin":     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
Step #4 - "test-quilkin": 
Step #4 - "test-quilkin": error: casting integer literal to `i64` is unnecessary
Step #4 - "test-quilkin":    --> src/filters/capture.rs:162:25
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin": 162 |                 "size": 3 as i64,
Step #4 - "test-quilkin":     |                         ^^^^^^^^ help: try: `3_i64`
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin":     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
Step #4 - "test-quilkin": 
Step #4 - "test-quilkin": error: useless conversion to the same type: `&str`
Step #4 - "test-quilkin":    --> src/xds/cluster.rs:785:38
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin": 785 |         let value = dyn_metadata.get("key-a".into()).unwrap();
Step #4 - "test-quilkin":     |                                      ^^^^^^^^^^^^^^ help: consider removing `.into()`: `"key-a"`
Step #4 - "test-quilkin":     |
Step #4 - "test-quilkin":     = note: `-D clippy::useless-conversion` implied by `-D warnings`
Step #4 - "test-quilkin":     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
Step #4 - "test-quilkin": 
Step #4 - "test-quilkin": error: could not compile `quilkin` due to 3 previous errors
Step #4 - "test-quilkin": make: *** [Makefile:55: test-quilkin] Error 101
Finished Step #4 - "test-quilkin"

@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: 3a8b1bcd-c679-464a-bb54-c17ed18004c4

To build this version:

git fetch [email protected]:googleforgames/quilkin.git pull/518/head:pr_518 && git checkout pr_518
cargo build

@XAMPPRocky XAMPPRocky merged commit 9d5f2da into main Apr 19, 2022
@markmandel markmandel deleted the rf/knock-down-serdeyaml branch May 18, 2022 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants