-
Notifications
You must be signed in to change notification settings - Fork 627
Replace Rake's Win32-specific logic with a 100% equivalent, pure-Ruby implementation #669
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
Conversation
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.
It is worth noting that the Win32HomeError exception is never thrown IRL, because Ruby will always set the HOME environment variable in its environment, even if it isn't set in the Windows environment the Ruby process is running in.
It has done so since Ruby 1.9.x in 2004: ruby/ruby@c41cefd492
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.
As per the above comment, this test is superfluous, as the exception will never be thrown IRL.
Uh oh!
There was an error while loading. Please reload this page.
tl;dr - replace some Win32-specific Rake logic for Windows home directory detection logic with Ruby's built-in
Dir.homemethodRake's custom Windows-specific implementation (
Rake::Win32::win32_system_dir) produces identical results to Ruby's standard library approach (File.join(Dir.home, "Rake")) for all possible scenarios it caters for, so this PR removes a whole bunch of superfluous code and corresponding tests.It is replaced with a much simpler, pure stdlib-based implementation. in a low-risk manner: Ruby's cross-platform
Dir.homehas been available since Ruby 1.9 and is the standard way to get the home directory; this change simply stops reinventing the wheel. 😃Because it's hard to use and test something that isn't there anymore 😄 I've raised an accompanying DRAFT pull request #670 that explicitly tests the equivalency of both versions against the master branch (for a variety of different scenarios) thereby demonstrating that the Rake-specific patch is superfluous and can be safely removed.