Skip to content

node: require LocalAppData variable#19132

Merged
karalabe merged 2 commits intoethereum:masterfrom
fjl:node-localappdata-required
Feb 28, 2019
Merged

node: require LocalAppData variable#19132
karalabe merged 2 commits intoethereum:masterfrom
fjl:node-localappdata-required

Conversation

@fjl
Copy link
Copy Markdown
Contributor

@fjl fjl commented Feb 19, 2019

This avoids path inconsistencies on Windows XP.
Hat tip to @MicahZoltu for catching this so quickly.

This avoids path inconsistencies on Windows XP.
Hat tip to @MicahZoltu for catching this so quickly.
@fjl fjl added this to the 1.9.0 milestone Feb 19, 2019
fjl referenced this pull request Feb 19, 2019
This changes default location of the data directory to use the LOCALAPPDATA
environment variable, resolving issues with remote home directories an improving
compatibility with Cygwin.

Fixes #2239 
Fixes #2237 
Fixes #16437
@MicahZoltu
Copy link
Copy Markdown

Minor comment. This will result in a panic on Windows XP, but it will work for everyone on Windows 7+. If you don't otherwise support Windows XP (not sure why you would at this point, it is really old and end of lifed by Microsoft), then this change is great.

@MicahZoltu
Copy link
Copy Markdown

Sorry for being unclear (and spamming your inbox!). Comment is correct, feedback about panic on XP was just related to PR description that said "avoids path inconsistencies on Windows XP", which the casual reader may interpret to mean "works on Windows XP". :)

Copy link
Copy Markdown
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

I'll believe you it's correct.

@karalabe karalabe merged commit dd28ba3 into ethereum:master Feb 28, 2019
hackmod pushed a commit to Ethersocial/go-ethersocial that referenced this pull request Apr 14, 2019
* node: require LocalAppData variable

This avoids path inconsistencies on Windows XP.
Hat tip to @MicahZoltu for catching this so quickly.

* node: fix typo
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 13, 2024
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