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

[Metricbeat] Migrate NGINX stubstatus to use ReporterV2 interface #11051

Merged
merged 2 commits into from
Mar 13, 2019
Merged

[Metricbeat] Migrate NGINX stubstatus to use ReporterV2 interface #11051

merged 2 commits into from
Mar 13, 2019

Conversation

berfinsari
Copy link
Contributor

@berfinsari berfinsari commented Mar 3, 2019 •
edited by alvarolobato
Loading

Refer to #10774 for more info

@berfinsari berfinsari requested a review from a team as a code owner March 3, 2019 19:43
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@sayden
Copy link
Contributor

sayden commented Mar 5, 2019

jenkins, test this

Thank you for contributing with this @berfinsari !

@sayden sayden added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Mar 5, 2019
@sayden sayden mentioned this pull request Mar 5, 2019
Closed
62 tasks
@ruflin
Copy link
Member

ruflin commented Mar 6, 2019

jenkins, test this

sayden
sayden approved these changes Mar 6, 2019
View reviewed changes
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

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

Tested locally. Everything looks good apart from discovering that we don't have system tests for this module and that file stubstatus.go had execution permissions (-rwxr-xr-x). @berfinsari would you mind to also update this in this PR too, please?

@sayden
Copy link
Contributor

sayden commented Mar 6, 2019 •
edited
Loading

Sorry, I mean to update the permissions file. System tests for this module will be a bit more laborious and it's better that it goes in a new PR

@berfinsari berfinsari force-pushed the migration/mb/reporterv2/nginx/stubstatus branch from 3049ccb to 3638e6e Compare March 10, 2019 17:27
@berfinsari
Copy link
Contributor Author

Thanks for the review! I may have overlooked this issue. I updated the permissions file. Is there anything else I need to do?

@ruflin
Copy link
Member

ruflin commented Mar 12, 2019

jenkins, test this

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

@ruflin
Copy link
Member

ruflin commented Mar 13, 2019

jenkins, test this

@ruflin ruflin merged commit f13cd2f into elastic:master Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sayden sayden sayden approved these changes

Assignees
No one assigned
Labels
Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@berfinsari @elasticmachine @sayden @ruflin @kaiyan-sheng