Skip to content
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

Support Rails 6.1.6.1 #1104

Merged
merged 1 commit into from
Aug 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions lib/suspenders/actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,27 @@ def expand_json(file, data)
action ExpandJson.new(destination_root, file, data)
end

def gem(*args)
options = args.extract_options!
name, *versions = args

parts = [quote(name)]

if (versions = versions.any? ? versions : options.delete(:version))
versions = Array(versions)
versions.each do |version|
parts << quote(version)
end
end

parts << quote(options) unless options.empty?

str = []
str << indentation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but I prefer how it reads in the original method:

    str = "gem #{parts.join(", ")}"
    str = indentation + str

Also, the original method uses in_root, but I suppose that is not necessary here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oddly enough, I was getting failures when I used that implementation, but I'm not totally sure why 🤔

I think this might have also been the result of having run standardrb --fix.

str << "gem #{parts.join(", ")}"
append_file "Gemfile", %(\n#{str.join}\n), verbose: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original Rails method uses append_file_with_newline. Is that method not reversible while append_file is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append_file_with_newline calls gsub_file which unfortunately is not reversible.

end

class ExpandJson
def initialize(destination_root, file, data)
@destination_root = destination_root
Expand Down
6 changes: 3 additions & 3 deletions lib/suspenders/app_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def configure_local_mail

def setup_asset_host
replace_in_file "config/environments/production.rb",
"# config.action_controller.asset_host = 'http://assets.example.com'",
'config.action_controller.asset_host = ENV.fetch("ASSET_HOST", ENV.fetch("APPLICATION_HOST"))'
"# config.asset_host = 'http://assets.example.com'",
'config.asset_host = ENV.fetch("ASSET_HOST", ENV.fetch("APPLICATION_HOST"))'

if File.exist?("config/initializers/assets.rb")
replace_in_file "config/initializers/assets.rb",
Expand Down Expand Up @@ -250,7 +250,7 @@ def development_env?
end

def raise_on_missing_translations_in(environment)
config = "config.action_view.raise_on_missing_translations = true"
config = "config.i18n.raise_on_missing_translations = true"

uncomment_lines("config/environments/#{environment}.rb", config)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/suspenders/generators/advisories_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Suspenders
class AdvisoriesGenerator < Generators::Base
def bundler_audit_gem
gem "bundler-audit", ">= 0.7.0", require: false, group: %i[development test]
gem "bundler-audit", ">= 0.7.0", require: false, group: [:development, :test]
Bundler.with_unbundled_env { run "bundle install" }
end

Expand Down
2 changes: 1 addition & 1 deletion lib/suspenders/generators/jobs_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Suspenders
class JobsGenerator < Generators::Base
def add_jobs_gem
append_file "Gemfile", %(\ngem "sidekiq"\n)
gem "sidekiq"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my fault. I was accidentally on main when I pushed that up 🤦.

Is it worth removing that commit before I merge this in?

Also, I wonder if we should enable a branch protection rule to prevent this from happening again?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your choice!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think branch protection would be great! As for the commit, it's small and harmless, so I think we can leave it as-is. This PR makes a change to the same line, and also improves it, so that's good.

Bundler.with_unbundled_env { run "bundle install" }
end

Expand Down
2 changes: 1 addition & 1 deletion lib/suspenders/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module Suspenders
RAILS_VERSION = "~> 6.0.0".freeze
RAILS_VERSION = "~> 6.1.6.1".freeze
RUBY_VERSION = IO
.read("#{File.dirname(__FILE__)}/../../.ruby-version")
.strip
Expand Down
2 changes: 1 addition & 1 deletion spec/features/new_project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
it "raises on missing translations in development and test" do
[development_config, test_config].each do |environment_file|
expect(environment_file).to match(
/^ +config.action_view.raise_on_missing_translations = true$/
/^ +config.i18n.raise_on_missing_translations = true$/
)
end
end
Expand Down
1 change: 1 addition & 0 deletions templates/active_job.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "active_job/logging"
require "active_job/log_subscriber"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this also necessary for Rails 6.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oddly enough, yes. Without it, we get uninitialized constant ActiveJob::Logging::LogSubscriber.


ActiveSupport::Notifications.unsubscribe("enqueue.active_job")

Expand Down