Skip to content

Implemented comma, equal sign and space escaping #74

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

Merged
merged 1 commit into from
Apr 15, 2017

Conversation

Zwartpet
Copy link

@Zwartpet Zwartpet commented Apr 7, 2017

@wdalmut
Copy link
Member

wdalmut commented Apr 10, 2017

Thanks for your PR.

we will review this asap.

At first sight i think that this escape procedure should go in the WriterTrait and not in the Client.

@Zwartpet
Copy link
Author

@wdalmut Moved it to the WriterTrait as requested ;)

Copy link
Member

@wdalmut wdalmut left a comment

Choose a reason for hiding this comment

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

Restore Client implementation (logic moved to WriterTrait)

src/Client.php Outdated
$data['points'][0]['fields'] = $values;
}

if (isset($data['tags'])) {
Copy link
Member

Choose a reason for hiding this comment

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

This file should be reverted to the origin/master version

@Zwartpet
Copy link
Author

Ah, didn't pay attention there, reverted the client.php to master.

@wdalmut
Copy link
Member

wdalmut commented Apr 12, 2017

I think that is all good. Can you just squash your 4 commits to a single commit?

@wdalmut wdalmut self-assigned this Apr 12, 2017
@Zwartpet Zwartpet force-pushed the patch/line-protocol branch from 27ad695 to e90e2bd Compare April 13, 2017 13:25
@scrutinizer-notifier
Copy link

The inspection completed: 1 updated code elements

@Zwartpet
Copy link
Author

I certainly can

@wdalmut wdalmut merged commit 7ea56e1 into corley:master Apr 15, 2017
@wdalmut
Copy link
Member

wdalmut commented Apr 15, 2017

👍 thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants