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

fix(recv-link): distinguish auto/manual settlement #317

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dnwe
Copy link
Contributor

@dnwe dnwe commented May 8, 2017

This should probably be a major version bump as the API is changed

  • default to rcv-settle-mode "first" everywhere, we don't currently handle
    "second" correctly anyway
  • add link policy attribute "settlement" to distinguish between auto,
    where the client will immediately accept messages on receipt, and manual
    where the user is expected to call accept when they have finished
    handling a delivery.

Fixes #316


This change is Reviewable

- default to rcv-settle-mode "first" everywhere, we don't currently handle
  "second" correctly anyway
- add link policy attribute "settlement" to distinguish between auto,
  where the client will immediately accept messages on receipt, and manual
  where the user is expected to call accept when they have finished
  handling a delivery.

Fixes noodlefrenzy#316
@@ -192,7 +192,7 @@ ReceiverLink.prototype._messageReceived = function(transferFrame) {
// @todo: Bump link credit based on strategy

// respect settle mode in policy
if (this.policy.attach.rcvSettleMode === constants.receiverSettleMode.autoSettle) {
if (this.policy.settlement === constants.settlement.manual) {
Copy link

@mmaestri mmaestri Jun 15, 2017

Choose a reason for hiding this comment

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

actually this should be
if (this.policy.settlement === constants.settlement.auto) {

@amarzavery amarzavery closed this Jan 23, 2018
@amarzavery amarzavery reopened this Jan 23, 2018
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