-
Notifications
You must be signed in to change notification settings - Fork 384
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
Resolve Rails 8 upgrade issues #614
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,30 @@ | ||
#!/usr/bin/env ruby | ||
exec "./bin/rails", "server", *ARGV | ||
# frozen_string_literal: true | ||
|
||
def installed?(process) | ||
IO.popen "#{process} -v" | ||
rescue Errno::ENOENT | ||
false | ||
end | ||
|
||
def run(process) | ||
system "#{process} start -f Procfile.dev" | ||
rescue Errno::ENOENT | ||
warn <<~MSG | ||
ERROR: | ||
Please ensure `Procfile.dev` exists in your project! | ||
MSG | ||
exit! | ||
end | ||
|
||
if installed? "overmind" | ||
run "overmind" | ||
elsif installed? "foreman" | ||
run "foreman" | ||
else | ||
warn <<~MSG | ||
NOTICE: | ||
For this script to run, you need either 'overmind' or 'foreman' installed on your machine. Please try this script after installing one of them. | ||
MSG | ||
exit! | ||
end |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -13,8 +13,12 @@ FileUtils.chdir APP_ROOT do | |||||||||||||||||||||||||
# Add necessary setup steps to this file. | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
puts "== Installing dependencies ==" | ||||||||||||||||||||||||||
system! "gem install bundler --conservative" | ||||||||||||||||||||||||||
system("bundle check") || system!("bundle install") | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
# Install JavaScript dependencies | ||||||||||||||||||||||||||
system! "bin/yarn" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add Node.js and Yarn version verification. Verify Node.js and Yarn versions meet the minimum requirements for Rails 8. # Install JavaScript dependencies
+ required_node_version = "18.0.0"
+ required_yarn_version = "1.22.0"
+
+ puts "\n== Verifying Node.js and Yarn versions =="
+ system! "node -v | grep -q 'v#{required_node_version}' || (echo 'Node.js #{required_node_version}+ is required'; exit 1)"
+ system! "yarn -v | grep -q '#{required_yarn_version}' || (echo 'Yarn #{required_yarn_version}+ is required'; exit 1)"
+
system! "bin/yarn" 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
# puts "\n== Copying sample files ==" | ||||||||||||||||||||||||||
# unless File.exist?("config/database.yml") | ||||||||||||||||||||||||||
# FileUtils.cp "config/database.yml.sample", "config/database.yml" | ||||||||||||||||||||||||||
|
This file was deleted.
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.
🛠️ Refactor suggestion
Enhance security of process version check.
The current implementation using
IO.popen
could be vulnerable to command injection. Consider usingwhich
command instead.