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.

Sign up for GitHub

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

Jump to bottom

Updated for NATS 2.0, added README for benchmarks #274

Merged
merged 1 commit into from
Jul 10, 2019
Merged

Updated for NATS 2.0, added README for benchmarks #274

merged 1 commit into from
Jul 10, 2019

Conversation

derekcollison
Copy link
Member

Signed-off-by: Derek Collison derek@nats.io

@coveralls
Copy link

coveralls commented Jul 9, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 93.886% when pulling e799d41 on updates into c5efdc3 on master.

kozlovic
kozlovic requested changes Jul 10, 2019
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

I think the -unsubscribe -> -unsub needs some changes. Otherwise, just wondering about the change from sync to async and receive new to all and how easy user can restore old behavior.

examples/stan-bench/main.go Outdated Show resolved Hide resolved
examples/stan-sub/main.go Show resolved Hide resolved
examples/stan-sub/main.go Show resolved Hide resolved
kozlovic
kozlovic requested changes Jul 10, 2019
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

I think creds are missing from flag parsing in stan-sub.

examples/stan-sub/main.go Show resolved Hide resolved
examples/stan-bench/main.go Outdated Show resolved Hide resolved
examples/stan-sub/main.go Show resolved Hide resolved
kozlovic
kozlovic approved these changes Jul 10, 2019
View reviewed changes
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison
Updated for NATS 2.0, added README for benchmarks
e799d41 
Signed-off-by: Derek Collison <derek@nats.io>
@derekcollison derekcollison merged commit fe52e0d into master Jul 10, 2019
@derekcollison derekcollison deleted the updates branch July 10, 2019 00:51
kozlovic added a commit that referenced this pull request Jul 24, 2019
@kozlovic
Fixed examples/stan-sub
62bcb6f 
In PR #274, some changes were made which caused stan-sub to be
a queue group by default, but then there is no way to make it
a plain subscription. Reverting this change.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic kozlovic mentioned this pull request Jul 24, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kozlovic kozlovic kozlovic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@derekcollison @coveralls @kozlovic