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.

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

Passes default locale setting when executing commands. #1353

Merged
merged 4 commits into from Oct 8, 2020

Conversation

dmitraver
Copy link
Contributor

Closes #1347

Passes locale when executing commands on different platforms.

@coveralls
Copy link

coveralls commented Oct 7, 2020

Coverage Status

Coverage decreased (-0.1%) to 87.975% when pulling 79fba26 on dmitraver:run-native-locale-fix-1347 into 493d109 on oshi:master.

Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Thanks. This works but I've left some comments on improving it.

Can you also add a change log entry under 5.3.0?

oshi-core/src/main/java/oshi/util/ExecutingCommand.java Outdated Show resolved Hide resolved
oshi-core/src/main/java/oshi/util/ExecutingCommand.java Outdated Show resolved Hide resolved
oshi-core/src/main/java/oshi/util/ExecutingCommand.java Outdated Show resolved Hide resolved
@dbwiddis dbwiddis added hacktoberfest-accepted Accepted during Hacktoberfest i18n Internationalization and Localization labels Oct 7, 2020
@XakepSDK
Copy link
Contributor

XakepSDK commented Oct 7, 2020

Maybe helper method? ENVP_WINDOWS and ENVP_UNIX constants.

private static String[] getDefaultEnv() {
    if (platform == PlatformEnum.WINDOWS) {
        return ENVP_WINDOWS;
    } else if (platform != PlatformEnum.UNKNOWN) {
        return ENVP_UNIX;
    } else {
        return null;
    }
}

@dbwiddis
Copy link
Member

dbwiddis commented Oct 7, 2020

Maybe helper method?

In the direction I was thinking but this will not change during operation, so we just need a one-time constant (So using this helper method but defining a constant as the result would work).

@dmitraver
Copy link
Contributor Author

Pushed review fixes.

@dmitraver
Copy link
Contributor Author

Ops, haven't seen your comment about changelog, adding it.

@dmitraver
Copy link
Contributor Author

@dbwiddis updated changelog and rebased.

@dbwiddis
Copy link
Member

dbwiddis commented Oct 8, 2020

This looks good! I'm headed to bed but will get this merged tomorrow. Thanks for your contribution!

@dbwiddis dbwiddis merged commit f393e69 into oshi:master Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted during Hacktoberfest i18n Internationalization and Localization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runNative locale issues on linux
4 participants