refactor: move platform detection to Rake module, and delete Rake::Win32 module#729
Open
pvdb wants to merge 1 commit intoruby:masterfrom
Open
refactor: move platform detection to Rake module, and delete Rake::Win32 module#729pvdb wants to merge 1 commit intoruby:masterfrom
Rake module, and delete Rake::Win32 module#729pvdb wants to merge 1 commit intoruby:masterfrom
Conversation
The Rake::Win32 module existed solely to answer one question: are we running on Windows? That single-method module was more ceremony than it was worth, and it meant platform detection was split across two places — Win32.windows? in win32.rb and Application#unix? in application.rb. Both methods now live together as Rake.windows? and Rake.unix? on the Rake module itself, which is the natural home for gem-level helpers. Application#windows? and Application#unix? are kept as thin delegators so nothing calling them through an application instance breaks.
9ac65f4 to
e4383ab
Compare
pvdb
commented
Apr 20, 2026
| # be both Windows and Unix at the same time | ||
| assert ! (Rake.windows? && Rake.unix?) | ||
| end | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This preserves the original TestRakeApplication.test_windows exclusivity check, but - arguably - with a more intent-revealing name, and also by documenting it (as the original assertion was somewhat esoteric) 😃
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
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.
The
Rake::Win32module existed solely to answer one question: are we running on Windows?That single-method module was more ceremony than it was worth, and it meant platform detection was split across two places —
Win32.windows?in win32.rb andApplication#unix?in application.rb.Both methods now live together as
Rake.windows?andRake.unix?on the Rake module itself, which is the natural home for gem-level helpers.Application#windows?andApplication#unix?are kept as thin delegators so nothing calling them through an application instance breaks.Beyond tidying up the code, the refactoring also untangles two distinct uses of platform detection that were previously conflated. 🎉
The first is a static, load-time concern: deciding which tests to define or which assertions to run. This only needs a simple module-level predicate — call it once when the class loads and you're done.
The second is a dynamic, instance-level concern: branching on platform inside
Applicationmethods, and — critically — being stubbable in tests so that platform-specific code paths can be exercised regardless of where CI runs.These two uses have different requirements.
Rake.windows?/Rake.unix?module methods serve the first cleanly.Applicationpreserve the second — including the ability to stub@app.windows?/@app.unix?in tests, which several existing tests rely on.I hope you like it, @hsbt 🤞 - this isn't just cleanup, it's a separation of concerns that makes the intent of each call site
clearer.