Skip to content

Conversation

@dantswain
Copy link
Collaborator

See #253 for discussion. There are changes to produce.ex here but they
are only formatting and refactoring that made it easier to reason about
this problem.

See #253 for discussion.  There are changes to produce.ex here but they
are only formatting and refactoring that made it easier to reason about
this problem.
def create_request(
correlation_id,
client_id,
request = %Request{compression: compression, messages: messages}

Choose a reason for hiding this comment

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

File has the variable name before the pattern while most of the files have the variable name after the pattern when naming parameter pattern matches

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 1 possible new issue (including those that may have been commented here).
  • 1 fixed issue! 🎉

You can see more details about this review at https://ebertapp.io/github/kafkaex/kafka_ex/pulls/256.

message_set
) do
KafkaEx.Protocol.create_request(:produce, correlation_id, client_id) <>
<< required_acks :: 16-signed, timeout :: 32-signed, 1 :: 32-signed >> <>
Copy link
Member

Choose a reason for hiding this comment

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

weird indentation here

Copy link
Member

@joshuawscott joshuawscott left a comment

Choose a reason for hiding this comment

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

Seems ok to me, a little ugly, but not sure what else we'd do.

@dantswain dantswain merged commit a63f6ea into master Nov 30, 2017
@dantswain dantswain deleted the gzip_error branch November 30, 2017 17:00
robotarmy pushed a commit to RAM9/kafka_ex that referenced this pull request Apr 7, 2025
Fix gzip test wrt changing versions of libz
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.

3 participants