-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Add forwarder demo #1669
Add forwarder demo #1669
Conversation
nng_socket sock_back_end = NNG_SOCKET_INITIALIZER; | ||
int ret = 0; | ||
|
||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a super silly nit, but I don't use these style of block comments anywhere else in this code base.
If you're willing, I'd appreciate if you could convert them to C++ style just for consistency.
I won't hold the PR up for this, because at least on the face of it, it is very nice and I'd like to see it land before the next release, but I will wait to give you a chance to respond (and possibly update) before I merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Will change them to match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulo stylistic nits, it looks great.
Still if you're willing to make the minor style fixes for consistency, I'd be even more grateful.
|
||
void panic_on_error(int should_panic, const char* format, ...) | ||
{ | ||
if (should_panic) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use clang-format using the .clang-format I've supplied, you'll see that for if statements like this the { should be on the same line.
Again, this is a very minor nit, but I'd be even happier with this if you could make it match the existing style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do.
@@ -0,0 +1,62 @@ | |||
= PubSub Forwarder | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent documentation for this addition! Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I tried to follow the other demos as an example.
I will try to merge this within the week, or sooner if you respond and ping me. :-) The only reason I'm not merging now is to give you a chance to make it even better by addressing the stylistic/consistency nits I pointed out. :-) |
I think that last commit takes care of the nits that needed picking. Let me know if there is anything else. |
This provides a demo for using nng_device to create a forwarder or proxy.