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.

Sign up for GitHub

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

Jump to bottom

Avoid double slash when join url and path (#5503) #5517

Merged
merged 5 commits into from
Nov 9, 2017
Merged

Avoid double slash when join url and path (#5503) #5517

merged 5 commits into from
Nov 9, 2017

Conversation

liketic
Copy link

@liketic liketic commented Nov 6, 2017

Closes #5503

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

ruflin
ruflin reviewed Nov 6, 2017
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Could you add a test for addToURL that confirms that both options work? Could you also add an entry to the Changelog?

libbeat/outputs/elasticsearch/client.go
@@ -84,7 +84,7 @@ var (
errExpectedItemObject = errors.New("expected item response object")
errExpectedStatusCode = errors.New("expected item status code")
errUnexpectedEmptyObject = errors.New("empty object")
errExcpectedObjectEnd = errors.New("expected end of object")
errExpectedObjectEnd = errors.New("expected end of object")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the cleanup

@ruflin ruflin requested a review from andrewkroh November 6, 2017 10:05
@liketic
Copy link
Author

liketic commented Nov 7, 2017

@ruflin Thanks. I pushed 734ad81

ruflin
ruflin approved these changes Nov 7, 2017
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

WFG

@ruflin
Copy link
Contributor

ruflin commented Nov 7, 2017

@liketic It seems you have to run make fmt on this PR. I pushed 2 additional commits to fix the merge conflict in the CHANGELOG as I hoped to directly merge. You need to pull these changes or just overwrite them if you push again.

@liketic
Copy link
Author

liketic commented Nov 8, 2017

@ruflin Thanks for your kind help. I think everything is OK now. 👍

@ruflin
Copy link
Contributor

ruflin commented Nov 8, 2017

jenkins, test it

@ruflin ruflin merged commit 67e05fc into elastic:master Nov 9, 2017
@liketic liketic deleted the enhancement/issues/5503 branch November 9, 2017 05:20
@damianpfister damianpfister mentioned this pull request Apr 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

@andrewkroh andrewkroh Awaiting requested review from andrewkroh

Assignees
No one assigned
Labels
libbeat review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@liketic @elasticmachine @ruflin @monicasarbu