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

updates rhea-promise to 2.0.0 to match rhea dependency being updated to 2.x #87

Merged
merged 5 commits into from
Jun 3, 2021

Conversation

chradek
Copy link
Collaborator

@chradek chradek commented Jun 3, 2021

Description

rhea has been updated to 2.x, so this PR updates rhea-promise to use rhea 2.x!

There was only one breaking change in rhea: deserializing timestamps as dates instead of numbers.

Since we're doing a major version bump of rhea-promise, I also updated AwaitableSender.send to move the optional positional arguments to the options argument.

@ramya-rao-a
Copy link
Collaborator

ramya-rao-a commented Jun 3, 2021

Now that we have a major version update allowing us to clean up APIs that would be breaking changes, can we do something about this: #66 (comment) and #66 (comment)

@ramya-rao-a
Copy link
Collaborator

Also, what do we have to gain by having timeout configurable both at the sender and each send level in the AwaitableSender? Can we afford to have it only on the send operation?

@chradek
Copy link
Collaborator Author

chradek commented Jun 3, 2021

Also, what do we have to gain by having timeout configurable both at the sender and each send level in the AwaitableSender? Can we afford to have it only on the send operation?

Sure! I think all it will mean is a few more changes to our client packages, but no issues with making this change.

@chradek
Copy link
Collaborator Author

chradek commented Jun 3, 2021

Now that we have a major version update allowing us to clean up APIs that would be breaking changes, can we do something about this: #66 (comment) and #66 (comment)

Done.

* @param {Buffer | string} [tag] The message tag if any.
* @param {number} [format] The message format. Specify this if a message with custom format needs
* to be sent. `0` implies the standard AMQP 1.0 defined format. If no value is provided, then the
* given message is assumed to be of type Message interface and encoded appropriately.
* @param {AwaitableSendOptions} [options] Options to configure the timeout and cancellation for
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we indicate that AwaitableSendOptions is now much more than timeout and cancellation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. Also made the same change for the regular Sender.

Copy link
Collaborator

@richardpark-msft richardpark-msft left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward.

@chradek chradek merged commit 93961ff into amqp:master Jun 3, 2021
@chradek chradek deleted the update-deps branch June 3, 2021 23:49
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