ruby-amqp / amq-protocol

AMQP 0.9.1 protocol serialization and deserialization implementation for Ruby (2.0+)
http://groups.google.com/group/ruby-amqp
MIT License
48 stars 31 forks source link

⚠️ Fixes for Ruby Warnings #62

Closed amatsuda closed 7 years ago

amatsuda commented 7 years ago

Here are fixes for some Ruby warnings with the 2.0.1 gem.

michaelklishin commented 7 years ago

Thank you for looking into this!

amatsuda commented 7 years ago

Ugh, why did I miss the comment at the top of the file I was editing... I guess I'm done!

michaelklishin commented 7 years ago

Now client.rb needs regenerating. I'm happy to do it myself, can you please change your branch name to something easier to work with in the shell?

amatsuda commented 7 years ago

I did run ./generate.rb and included the related change for client.rb in the relevant commit.

Besides that, I see a chunk of diff like this in the generated code:

diff --git a/lib/amq/protocol/client.rb b/lib/amq/protocol/client.rb
index 85dafb7..60a1c51 100644
--- a/lib/amq/protocol/client.rb
+++ b/lib/amq/protocol/client.rb
@@ -1437,8 +1437,7 @@ module AMQ
         result = [60, 0].pack(PACK_UINT16_X2)
         result += AMQ::Pack.pack_uint64_big_endian(body_size)
         result += [flags].pack(PACK_UINT16)
-        pieces_joined = pieces.join(EMPTY_STRING)
-        result.force_encoding(pieces_joined.encoding) + pieces_joined
+        result + pieces.join(EMPTY_STRING)
       end

       # THIS DECODES ONLY FLAGS

I didn't include that in this PR because this reproduces when I run the generator in master, so I'm sure this is unrelated to my changes here.

michaelklishin commented 7 years ago

OK, I will take a look. Thank you.

michaelklishin commented 7 years ago

@amatsuda thank you. Indeed the fix in #56 wasn't properly applied to the pytemplate file.

A minor comment on the branch name if I may. It's certainly witty to use a single warning sign emoji in a branch that fixes warnings but it was a bit frustrating to work with in the terminal.

I will release a 2.1.0 later today or tomorrow. Thanks again.

amatsuda commented 7 years ago

@michaelklishin Thank you! And I'm so sorry about the branch name...