-
Notifications
You must be signed in to change notification settings - Fork 80
Fix improper quoting on tag values when the value was text #64
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
Fix improper quoting on tag values when the value was text #64
Conversation
Looks like there were clippy lint issues as part of master and I'm unsure if I should fix them. I also tried tests but those seem broken and unable to run as well? It's likely this change breaks a test though so if its required for merging, just going to need help getting it working so I can fix them (or at least a name of the broken test since the fix should be trivial). |
Thanks for fixing the issue 👍 🚀 There should have been a CI run showing which tests fail. I'll check what that didn't trigger. |
@sparky8251 I have added a new CI pipeline based on Github Actions and also fixed the clippy issues. Sadly I do not have maintainer write permissions to your fork, so it would be nice if you could rebase to the latest master and have CI tell us which tests are incorrect. |
9e87777
to
11b8c53
Compare
0117549
to
e821d83
Compare
Ok, All the clippy lints and tests pass now. The Squashed down all my test attempts so its just 2 discrete commits. If you want 1, let me know. |
Taking a look now! Thanks for rebasing!
Honestly InfluxDb line protocol is very painful. As this PR uncovered, my initial implementation is not correct even though I based the escaping parts and tests off of the official Go package. |
From what I can tell, the new behavior looks correct. All types of escaped tested in the go sdk are covered. Thanks for creating this PR and helping make this crate better. |
No problem! Using this crate to pull sensor data from embedded devices and this was something that bit me. Ended up making it incompatible with other libraries and methods of inputting data into influxdb so I had to fix it :) |
I think this change will be pushed to crates.io when #62 is merged. Until then, you reference this repo directly by Git. If you come across further problems, feel free to open issues or pull requests addressing them. All improvements appreciated. |
Description
Fixes #60
Checklist
cargo fmt --all
cargo clippy --all-targets --all-features -- -D warnings
cargo readme > README.md