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

Add response_time to monit plugin #8056

Merged
merged 7 commits into from
Nov 23, 2020
Merged

Add response_time to monit plugin #8056

merged 7 commits into from
Nov 23, 2020

Conversation

wowi42
Copy link
Contributor

@wowi42 wowi42 commented Aug 30, 2020

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

soufiane-bouchaara
soufiane-bouchaara approved these changes Aug 30, 2020
View reviewed changes
Copy link

@soufiane-bouchaara soufiane-bouchaara left a comment

Choose a reason for hiding this comment

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

Looks good to me , I was waiting for this update

@sjwang90 sjwang90 changed the title Improve monit Add response_time to monit plugin Aug 31, 2020
@sjwang90
Copy link
Contributor

@SirishaGopigiri if you'd like to take a look.

@sjwang90 sjwang90 added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Aug 31, 2020
@wowi42
Copy link
Contributor Author

wowi42 commented Aug 31, 2020

not sure if the failed test is linked to my update...

@srebhan srebhan mentioned this pull request Aug 31, 2020
Merged
3 tasks
@srebhan
Copy link
Member

srebhan commented Sep 1, 2020

Please rebase on master to fix the AppVeyor problem.

@wowi42
Copy link
Contributor Author

wowi42 commented Sep 7, 2020

@srebhan I think it's ok now, right?

@srebhan
Copy link
Member

srebhan commented Sep 7, 2020

@wowi42 if it builds it's ok! ;-)

@wowi42
Copy link
Contributor Author

wowi42 commented Sep 7, 2020

@srebhan It seems so, no?

@srebhan
Copy link
Member

srebhan commented Sep 7, 2020

Yes, it's building ok now.

@wowi42
Copy link
Contributor Author

wowi42 commented Oct 14, 2020

Any plan to merge it?

@sjwang90 sjwang90 requested a review from ssoroka October 14, 2020 18:20
ssoroka
ssoroka suggested changes Oct 14, 2020
View reviewed changes
Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

There seems to be something wrong with this PR, it has a bunch of unrelated changes.

@wowi42
Copy link
Contributor Author

wowi42 commented Nov 3, 2020

fixed @ssoroka

srebhan
srebhan approved these changes Nov 21, 2020
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@ssoroka seems like the unrelated changes are gone. LGTM.

@srebhan srebhan self-assigned this Nov 21, 2020
@ssoroka ssoroka merged commit d64c722 into influxdata:master Nov 23, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Nov 23, 2020

Thank you! merged

arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@wowi42
Add response_time to monit plugin (influxdata#8056)
7395fd4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssoroka ssoroka ssoroka approved these changes

@soufiane-bouchaara soufiane-bouchaara soufiane-bouchaara approved these changes

@srebhan srebhan srebhan approved these changes

Assignees

@srebhan srebhan

Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@wowi42 @sjwang90 @srebhan @ssoroka @soufiane-bouchaara