Skip to content

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

Merged
ruflin merged 2 commits intoelastic:masterfrom
berfinsari:migration/mb/reporterv2/nginx/stubstatus
Mar 13, 2019
Merged

[Metricbeat] Migrate NGINX stubstatus to use ReporterV2 interface#11051
ruflin merged 2 commits intoelastic:masterfrom
berfinsari:migration/mb/reporterv2/nginx/stubstatus

Conversation

@berfinsari
Copy link
Copy Markdown
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
Copy Markdown
Contributor

elasticmachine commented Mar 3, 2019

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
Copy Markdown
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
Copy Markdown
Contributor

ruflin commented Mar 6, 2019

jenkins, test this

sayden
sayden approved these changes Mar 6, 2019
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

berfinsari commented Mar 10, 2019

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
Copy Markdown
Contributor

ruflin commented Mar 12, 2019

jenkins, test this

@kaiyan-sheng
Copy link
Copy Markdown
Contributor

kaiyan-sheng commented Mar 13, 2019

jenkins, test this please

@ruflin
Copy link
Copy Markdown
Contributor

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

1 more reviewer

@sayden sayden sayden approved these changes

Reviewers whose approvals may not affect merge requirements

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