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

Add abstract definition for debug_message #1927

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

garymardell
Copy link
Contributor

@garymardell garymardell commented Oct 31, 2024

Purpose

To make it clearer that debug_message must be implemented on Response.

Description

I am using lucky to build a microservice that accepts protobuf definitions over http so I created my own ProtobufResponse class to stream the protobuf response straight into the response io. While implementing it I discovered that you also have to implement debug_message as well as it is required by renderable:

response.debug_message.try do |message|
Lucky::Log.debug { message }
end

Checklist

  • - An issue already exists detailing the issue/or feature request that this PR fixes
  • - All specs are formatted with crystal tool format spec src
  • - Inline documentation has been added and/or updated
  • - Lucky builds on docker with ./script/setup
  • - All builds and specs pass on docker with ./script/test

@garymardell garymardell force-pushed the response-debug-message branch from ff8794f to bd80fc7 Compare October 31, 2024 22:05
Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks 👍

@jwoertink jwoertink merged commit 921436f into luckyframework:main Oct 31, 2024
7 checks passed
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