Skip to content

Conversation

emilyst
Copy link

@emilyst emilyst commented May 6, 2024

Description

This change adds the #on_stopped method to Puma's DSL, to match the functionality provided by #on_booted. Since this DSL method is documented, its absence appears to be a simple oversight, and it seems appropriate to have.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

This change adds the `#on_stopped` method to Puma's DSL, to match the
functionality provided by `#on_booted`. Since this DSL method is
documented, its absence appears to be a simple oversight, and it seems
appropriate to have.
@johnnyshields
Copy link
Contributor

LGTM

@dentarg dentarg added feature waiting-for-review Waiting on review from anyone labels Jun 5, 2024
Comment on lines +722 to +723
@options[:on_stopped] ||= []
@options[:on_stopped] << block
Copy link

@pointlessone pointlessone Jun 10, 2024

Choose a reason for hiding this comment

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

This looks different to other events. And a simple test in config shows it's being ignored completely. On the other hand, delegating this to events seem to work.

Suggested change
@options[:on_stopped] ||= []
@options[:on_stopped] << block
@config.options[:events].on_stopped(&block)

Comment on lines +476 to +478
def test_run_hooks_on_stopped_hook
assert_run_hooks :on_stopped
end

Choose a reason for hiding this comment

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

Given the change above, the test should probably mirror test_on_booted instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature waiting-for-review Waiting on review from anyone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants