-
-
Notifications
You must be signed in to change notification settings - Fork 528
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
Support Rails 6.1.6.1 #1104
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 |
---|---|---|
|
@@ -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 | ||
str << "gem #{parts.join(", ")}" | ||
append_file "Gemfile", %(\n#{str.join}\n), verbose: false | ||
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. The original Rails method uses 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.
|
||
end | ||
|
||
class ExpandJson | ||
def initialize(destination_root, file, data) | ||
@destination_root = destination_root | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
module Suspenders | ||
class JobsGenerator < Generators::Base | ||
def add_jobs_gem | ||
append_file "Gemfile", %(\ngem "sidekiq"\n) | ||
gem "sidekiq" | ||
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. Beautiful! 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. That was my fault. I was accidentally on 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? 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. Your choice! 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. 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 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
require "active_job/logging" | ||
require "active_job/log_subscriber" | ||
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. Is this also necessary for Rails 6.1? 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. Oddly enough, yes. Without it, we get |
||
|
||
ActiveSupport::Notifications.unsubscribe("enqueue.active_job") | ||
|
||
|
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.
Not a big deal, but I prefer how it reads in the original method:
Also, the original method uses
in_root,
but I suppose that is not necessary here.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.
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
.