Skip to content

Correctly remove the buildin rails adapter#23

Merged
michaelklishin merged 1 commit intoruby-amqp:mainfrom
Earlopain:correctly-remove-rails-adapter
Nov 8, 2024
Merged

Correctly remove the buildin rails adapter#23
michaelklishin merged 1 commit intoruby-amqp:mainfrom
Earlopain:correctly-remove-rails-adapter

Conversation

@Earlopain
Copy link
Contributor

Followup #19. cc @dixpac

defined? for a string literal is always truthy, same as for symbols.
defined?(ActiveJob::QueueAdapters::SneakersAdapter) would work as an alternative.

module ActiveJob
  module QueueAdapters
    # Adapter removed from rails
  end
end

module ActiveJob
  module QueueAdapters
    remove_const(:SneakersAdapter) if defined?("::#{name}::SneakersAdapter")
  end
end

test.rb:10:in 'remove_const': constant ActiveJob::QueueAdapters::SneakersAdapter not defined (NameError)

`defined?` for a string literal is always truthy, same as for symbols.
`defined?(ActiveJob::QueueAdapters::SneakersAdapter)` would work as an alternative.

```rb
module ActiveJob
  module QueueAdapters
    # Adapter removed from rails
  end
end

module ActiveJob
  module QueueAdapters
    remove_const(:SneakersAdapter) if defined?("::#{name}::SneakersAdapter")
  end
end
```

test.rb:10:in `remove_const': constant ActiveJob::QueueAdapters::SneakersAdapter not defined (NameError)
@michaelklishin michaelklishin added this to the 3.1.2 milestone Nov 8, 2024
@michaelklishin michaelklishin merged commit 61b3d87 into ruby-amqp:main Nov 8, 2024
@michaelklishin
Copy link
Member

@Earlopain I assume we really need to produce a new patch release? I can do that today.

@Earlopain
Copy link
Contributor Author

Ah no, I don't think so. The adapter is still present in rails 8.0 so it is not urgent for a number of months yet. It isn't even deprecated yet (if they go with that route).

@michaelklishin
Copy link
Member

@Earlopain ah, OK. Thank you for the explanation. We certainly will ship a new patch release in the next few months.

@Earlopain Earlopain deleted the correctly-remove-rails-adapter branch November 8, 2024 16:01
michaelklishin pushed a commit that referenced this pull request Nov 8, 2024
`defined?` for a string literal is always truthy, same as for symbols.
`defined?(ActiveJob::QueueAdapters::SneakersAdapter)` would work as an alternative.

```rb
module ActiveJob
  module QueueAdapters
    # Adapter removed from rails
  end
end

module ActiveJob
  module QueueAdapters
    remove_const(:SneakersAdapter) if defined?("::#{name}::SneakersAdapter")
  end
end
```

test.rb:10:in `remove_const': constant ActiveJob::QueueAdapters::SneakersAdapter not defined (NameError)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants