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.

Sign up for GitHub

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

Jump to bottom

Allow connection option to be callable #29

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Allow connection option to be callable #29

merged 1 commit into from
Jan 24, 2025

Conversation

tie
Copy link

@tie tie commented Jan 24, 2025 •
edited
Loading

This change allows :connection option to be callable and avoid calling start on bunny sessions.

@tie tie marked this pull request as draft January 24, 2025 11:24
@tie tie changed the title Allow using existing bunny session without calling start Allow connection option to be callable Jan 24, 2025
@tie tie marked this pull request as ready for review January 24, 2025 11:54
michaelklishin
michaelklishin requested changes Jan 24, 2025
View reviewed changes
@michaelklishin
Copy link
Member

This is a sound improvement. Please address the two tiny feedback items.

@tie
Copy link
Author

tie commented Jan 24, 2025

Done.

@tie tie requested a review from michaelklishin January 24, 2025 13:15
@michaelklishin michaelklishin added this to the 3.2.0 milestone Jan 24, 2025
@michaelklishin
Copy link
Member

Looks like a mock-based test needs an update:

  1) Failure:
Sneakers::Publisher::#publish::externally instantiated bunny session#test_0002_can handle an existing connection that is online [vendor/bundle/ruby/3.4.0/gems/rr-3.1.1/lib/rr/expectations/times_called_expectation.rb:29]:
connected?()

@tie
Allow connection option to be callable
849b79d 
This change allows :connection option to be callable and avoid calling
start on bunny sessions.
@michaelklishin michaelklishin merged commit 8f1b066 into ruby-amqp:main Jan 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelklishin michaelklishin michaelklishin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@tie @michaelklishin