Skip to content

Conversation

@pvdb
Copy link
Contributor

@pvdb pvdb commented Oct 2, 2024

Due to it being a tad convoluted, it took me longer than it probably should have to grok what the Rake::Application#system_dir method was trying to do.

I think this simplification makes it a lot more obvious, especially as the same ...

hash[:lookup_key] || :default_value

... idiom is already used elsewhere in the rake codebase.

It also avoids a double hash lookup: one lookup to see if RAKE_SYSTEM is set in the ENV hash, and - if it is - exactly the same lookup again to actually fetch its value. 🎉

PS - I was tempted to also include the following change in this PR ... happy to take your steer @hsbt : include it in this one, or issue a separate PR instead? (or else forget about it altogether if it's totally bonkers?) 😅

--- a/lib/rake/application.rb
+++ b/lib/rake/application.rb
@@ -765,7 +765,7 @@ module Rake
       end
     else
       def standard_system_dir #:nodoc:
-        File.join(File.expand_path("~"), ".rake")
+        File.join(Dir.home, ".rake")
       end
     end
     private :standard_system_dir

@pvdb pvdb changed the title avoid double ENV["RAKE_SYSTEM"] lookup chore: simplify Rake::Application#system_dir method Oct 2, 2024
@hemalvarambhia
Copy link

👍

@pvdb pvdb changed the title chore: simplify Rake::Application#system_dir method refactor: simplify Rake::Application#system_dir method Dec 8, 2024
@hsbt hsbt merged commit 5625c0d into ruby:master Dec 9, 2024
@pvdb pvdb deleted the simplify_rake_system_dir branch December 9, 2024 10:45
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