fix: envconfig, use OS-specific config file paths#2762
Merged
Conversation
THardy98
changed the title
mjameswh
reviewed
Jan 15, 2026
| if (osName != null) { | ||
| String osNameLower = osName.toLowerCase(); | ||
| if (osNameLower.contains("mac")) { | ||
| return userDir + "/Library/Application Support/temporalio/temporal.toml"; |
Contributor
There was a problem hiding this comment.
nit: Not a big deal, but it's preferable to avoid building paths by simply concatenating raw strings.
Suggested change
| return userDir + "/Library/Application Support/temporalio/temporal.toml"; | |
| return Paths.get(userDir, "Library", "Application", "Support", "temporalio", "temporal.toml").toString(); |
Same for other below.
mjameswh
reviewed
Jan 15, 2026
| * Get the default config file path based on the operating system: | ||
| * | ||
| * <ul> | ||
| * <li>macOS: $HOME/Library/Application Support/temporalio/temporal.toml |
Contributor
There was a problem hiding this comment.
This comment is on a private method, where it is less likely to be found by a user that wonders where ClientConfig is storing its data, which is a typical and legitimate question for a user. I'd suggest moving this bullet list to the class's javadoc instead, then just add a reference to that here.
mjameswh
approved these changes
Jan 15, 2026
Contributor
mjameswh
left a comment
mjameswh
left a comment
There was a problem hiding this comment.
Minor comments, nothing blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Envconfig fix, to use OS-specific config file paths
Why?
This is the correct behaviour (as documented)
Closes Environment Configuration does not read the correct file path on macOS #2754
How was this tested:
Added test
Any docs updates needed?
No