Skip to content

Ping on Windows - Set codepage to English - #992

Merged
ChristineTChen merged 5 commits into
masterfrom
shigeya-dd-ping-1
Sep 24, 2021
Merged

Ping on Windows - Set codepage to English#992
ChristineTChen merged 5 commits into
masterfrom
shigeya-dd-ping-1

Conversation

@shigeya-dd

@shigeya-dd shigeya-dd commented Sep 10, 2021

Copy link
Copy Markdown
Contributor

What does this PR do?

On non-US Windows, output of the ping.exe is localized.
It breaks the ping check and results in error.
To avoid this, I added the code to set the code page to English.
chcp and code page: docs

Motivation

Feedback and support tickets from users.

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

On non-US Windows, output of the ping.exe is localized.
It breaks the ping check and results in error.
To avoid this, I added the code to set the code page to English.
chcp and code page: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/chcp
Adjusted styles
adjusted styles
adjusted styles

@ChristineTChen ChristineTChen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@shigeya-dd ,

thanks for the PR. I left a comment just to clean up the code a little bit. I'll still wait on merging until the maintainer has had a chance to review.

Comment thread ping/datadog_checks/ping/ping.py
Comment thread ping/datadog_checks/ping/ping.py Outdated
Comment thread ping/datadog_checks/ping/ping.py Outdated
Co-authored-by: Christine Chen <ChristineTChen@users.noreply.github.com>
@shigeya-dd

Copy link
Copy Markdown
Contributor Author

Thanks @ChristineTChen ! I cleaned up the codes.

@ChristineTChen ChristineTChen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Pinging @jstanton617 as the maintainer for a look as well

@ChristineTChen

Copy link
Copy Markdown
Contributor

Hi @jstanton617 , quick follow up on my previous ping.

@jstanton617 jstanton617 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@ChristineTChen
ChristineTChen merged commit 03e325c into master Sep 24, 2021
@ChristineTChen
ChristineTChen deleted the shigeya-dd-ping-1 branch September 24, 2021 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants