search

stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

Stomp::Client modifies given headers hash #125

Closed mikola-spb closed 8 years ago

mikola-spb commented 8 years ago

I've realised that Client modifies given headers hash, which I think is misleading. Usually I don't expect that a method modifies parameters, until it's clear from its name (like "merge!" or "check_and_add_something"). Is it by purpose? I'm fine to submit a PR to change that.

gmallard commented 8 years ago

Sure, a PR would be fine.

I am thinking a short example of that should be possible. I would like to see one if you can supply it.

There is no intent to modify headers. But there are really no tests for it either.

On 07/11/2016 12:47 PM, Nikolay Khasanov wrote:

I've realised that Client modifies given headers hash, which I think is misleading. Usually I don't expect that a method modifies parameters, until it's clear from its name (like "merge!" or "check_and_add_something"). Is it by purpose? I'm fine to submit a PR to change that.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stompgem/stomp/issues/125, or mute the thread https://github.com/notifications/unsubscribe/AAAbv5yAvAGSzmqJHmMbQc47PPmOLFsfks5qUnOOgaJpZM4JJjyC.

gmallard commented 8 years ago

Please close this if you are satisfied with the result.

This fix is on 'dev', and will be in the next release.

mikola-spb commented 8 years ago

Thank you! There is one more place I found, but it's connected with listener and I'm not sure it has to be changed. Anyway it another story.

gmallard commented 8 years ago

Thanks for closing this.

Since you are using Artemis I thought you might be interested in working on this:

https://github.com/stompgem/stomp/issues/128

Let me know please. I understand if you are busy.