Fix rails new --dev
#38463
Fix rails new --dev
#38463
Conversation
|
Is it feasible to add test coverage for the |
|
@eugeneius Something I considered was clearing However, looking at the test failures, it seems like this change has far-reaching implications. I'm not sure if it is a workable solution. |
|
Those test failures are also happening on master, I don't think they're caused by this change: |
|
@eugeneius 🙏 🙏 🙏 I saw Once master is fixed I will trigger another build to test these changes. Do you think I should add the change I mentioned regarding clearing |
Follow-up to rails#38429. `Rails::Command.invoke` passes arguments through to the appropriate command class. However, some command classes were ignoring those arguments, and instead relying on the contents of ARGV. In particular, RakeCommand expected ARGV to contain the arguments necessary to the Rake task, and no other arguments. This caused the `webpacker:install` task to fail when the `--dev` option from `rails new --dev` remained in ARGV. This commit changes the relevant command classes to not rely on the previous contents of ARGV. This commit also adds a missing `require` for use of `Kernel#silence_warnings`. Fixes rails#38459.
jonathanhefner
force-pushed
the
fix-rails-new-dev
branch
from
f5fcbf6
to
7e54d3b
Compare
|
Cool! Yeah, I'm not quite sure how to test this, I guess we can pass Clearing ARGV is interesting! It was definitely a nuisance when I wrote all this, so if we can find a standard way to handle it, that would be great 🙏 |
Follow-up to rails#38463. This commit changes additional command classes to not depend on prior ARGV contents.
Follow-up to rails#38463. By isolating ARGV, we guard against commands inadvertently depending on prior ARGV contents. Any such command will now behave consistently when run via `Rails::Command.invoke`, whether coming from the Rails CLI or from library code. Likewise, any ARGV mutations done by a command will not affect code that executes after `Rails::Command.invoke`.
Follow-up to #38429.
Rails::Command.invokepasses arguments through to the appropriate command class. However, some command classes were ignoring those arguments, and instead relying on the contents ofARGV. In particular,RakeCommandexpectedARGVto contain the arguments necessary to the Rake task, and no other arguments. This caused thewebpacker:installtask to fail when the--devoption fromrails new --devremained inARGV.This commit changes the relevant command classes to not rely on the previous contents of
ARGV. This commit also adds a missingrequirefor use ofKernel#silence_warnings.Fixes #38459.