-
Notifications
You must be signed in to change notification settings - Fork 222
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
Align windows agent to linux so that service is disabled on when datadog_enabled false #338
Align windows agent to linux so that service is disabled on when datadog_enabled false #338
Conversation
Disable Service if not datadog_enabled
…nto erikhjensen/dd-agent-disable-on-not-enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @erikhjensen,
Thanks for the PR! I left a few nits, but overall this looks good to me. I tested your changes on Windows and Linux and they're working fine.
Co-authored-by: Kylian Serrania <kylian.serrania@datadoghq.com>
…le (datadog_additional_checks) Co-authored-by: Kylian Serrania <kylian.serrania@datadoghq.com>
Co-authored-by: Kylian Serrania <kylian.serrania@datadoghq.com>
…/github.com/erikhjensen/ansible-datadog into erikhjensen/dd-agent-disable-on-not-enabled
Hi @KSerrania I did not perform a reverse pull request to update my fork w/ the nits. From your comments about testing I am either assuming the updates are innately benign or, that you did test them. Is there further action for me? |
Hey @erikhjensen, I think these updates are fine. I'll do one last round of testing tomorrow to double-check everything and then approve & merge the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'll merge once the linting error is fixed.
Co-authored-by: Kylian Serrania <kylian.serrania@datadoghq.com>
This includes several proposed modifications: