{"action":"created","issue":{"url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/399","repository_url":"https://api.github.com/repos/alanxz/rabbitmq-c","labels_url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/399/labels{/name}","comments_url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/399/comments","events_url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/399/events","html_url":"https://github.com/alanxz/rabbitmq-c/pull/399","id":200483603,"number":399,"title":"tutorial 1 and 2","user":{"login":"apre","id":1763378,"avatar_url":"https://avatars.githubusercontent.com/u/1763378?v=3","gravatar_id":"","url":"https://api.github.com/users/apre","html_url":"https://github.com/apre","followers_url":"https://api.github.com/users/apre/followers","following_url":"https://api.github.com/users/apre/following{/other_user}","gists_url":"https://api.github.com/users/apre/gists{/gist_id}","starred_url":"https://api.github.com/users/apre/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/apre/subscriptions","organizations_url":"https://api.github.com/users/apre/orgs","repos_url":"https://api.github.com/users/apre/repos","events_url":"https://api.github.com/users/apre/events{/privacy}","received_events_url":"https://api.github.com/users/apre/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-01-12T21:24:19Z","updated_at":"2017-02-07T06:12:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/alanxz/rabbitmq-c/pulls/399","html_url":"https://github.com/alanxz/rabbitmq-c/pull/399","diff_url":"https://github.com/alanxz/rabbitmq-c/pull/399.diff","patch_url":"https://github.com/alanxz/rabbitmq-c/pull/399.patch"},"body":"Hi,\r\nHere are the tutorial 1 - and 2 example files, with now a compatible license.\r\n\r\nkind regards,\r\n\r\nAdrien\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/alanxz/rabbitmq-c/399)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/comments/277910854","html_url":"https://github.com/alanxz/rabbitmq-c/pull/399#issuecomment-277910854","issue_url":"https://api.github.com/repos/alanxz/rabbitmq-c/issues/399","id":277910854,"user":{"login":"alanxz","id":439940,"avatar_url":"https://avatars.githubusercontent.com/u/439940?v=3","gravatar_id":"","url":"https://api.github.com/users/alanxz","html_url":"https://github.com/alanxz","followers_url":"https://api.github.com/users/alanxz/followers","following_url":"https://api.github.com/users/alanxz/following{/other_user}","gists_url":"https://api.github.com/users/alanxz/gists{/gist_id}","starred_url":"https://api.github.com/users/alanxz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alanxz/subscriptions","organizations_url":"https://api.github.com/users/alanxz/orgs","repos_url":"https://api.github.com/users/alanxz/repos","events_url":"https://api.github.com/users/alanxz/events{/privacy}","received_events_url":"https://api.github.com/users/alanxz/received_events","type":"User","site_admin":false},"created_at":"2017-02-07T06:12:18Z","updated_at":"2017-02-07T06:12:18Z","body":"\n\n\n\nReview status: 0 of 5 files reviewed at latest revision, 15 unresolved discussions.\n\n---\n\n*[a discussion](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMBfSuqDblgPD93JNm:-KcMBfSvHWvdeL554WoB:b-bzyblp) (no related file):*\nApologies for taking so long to get to this, this is really a fantastic start.\n\nFirst round of comments here.\n\n---\n\n*[examples/new_task.c, line 17 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM5vda7QDhiR3TEMOj:-KcM5vda7QDhiR3TEMOk:bwnsbvs) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L17)):*\n>  * Portions created by Alan Antonuk are Copyright (c) 2012-2013\n>  * Alan Antonuk. All Rights Reserved.\n>  *\n>  * Portions created by Mike Steinert are Copyright (c) 2012-2013\n>  * Mike Steinert. All Rights Reserved.\n>  *\n>  * Portions created by VMware are Copyright (c) 2007-2012 VMware, Inc.\n>  * All Rights Reserved.\n>  *\n>  * Portions created by Tony Garnock-Jones are Copyright (c) 2009-2010\n>  * VMware, Inc. and Tony Garnock-Jones. All Rights Reserved.\n>  *\n\nYou can remove all of this, just leave your contribution (here and throughout).\n\n---\n\n*[examples/new_task.c, line 55 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMAzXCZn0rW5U2Rnor:-KcMAzXDhpBNeD9X0Ncs:blojjop) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L55)):*\n> ```C\n> #include <amqp_tcp_socket.h>\n> #include <amqp.h>\n> #include <amqp_framing.h>\n> ```\n\n`amqp.h` includes this. No need to include this here.\n\n---\n\n*[examples/new_task.c, line 63 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM66KHbkaHKOQmQb2-:-KcM66KHbkaHKOQmQb20:byd9u75) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L63)):*\n> ```C\n>     char const *hostname= \"localhost\";\n>     int port= 5672;\n>     int  status;\n> ```\n\nPlease format all of the source-code files with clang-format (https://clang.llvm.org/docs/ClangFormat.html).  The .clang-format file in the root of the repository should set the correct coding standard.\n\n---\n\n*[examples/new_task.c, line 74 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM6XGR08SyDEWHtaxH:-KcM6XGR08SyDEWHtaxI:b1vjtxv) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L74)):*\n> ```C\n>     amqp_rpc_reply_t r;\n> \n>     (void) argc; (void ) argv; // get rid of the unused variable warning\n> ```\n\nJust drop both of these from the function signature (here and throughout).\n\n---\n\n*[examples/new_task.c, line 86 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM6lxYWcLPnX4XvzKl:-KcM6lxYWcLPnX4XvzKm:b-m06inl) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L86)):*\n> ```C\n> \n>     r=amqp_login(conn, \"/\", 0, 131072, 0, AMQP_SASL_METHOD_PLAIN, \"guest\", \"guest\");\n>     assert(r.reply_type==AMQP_RESPONSE_NORMAL);\n> ```\n\nIdeally we shouldn't use `assert()`. \n\nUse an if-statement like whats done with `amqp_socket_open` above.\n\n---\n\n*[examples/new_task.c, line 88 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM7FjEYmuFPmoBTNuV:-KcM7FjEYmuFPmoBTNuW:bq9sfdg) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L88)):*\n> ```C\n>     assert(r.reply_type==AMQP_RESPONSE_NORMAL);\n> \n>     amqp_channel_open(conn, 1);\n> ```\n\nThe correct path for error-checking for RPC-type calls is to check for `NULL`, which indicates an error.  If the result is NULL, then use `amqp_get_rpc_reply()` to get the details of what failed (the existing examples are bad at this).\n\n```c\namqp_channel_open_ok_t* res = amqp_channel_open(conn, 1);\nif (res == NULL) {\n  /* Use amqp_get_rpc_reply() to get details on what failed */\n  printf(\"failed to open channel\\n\");\n  exit(1);\n...\n ```\n\n---\n\n*[examples/new_task.c, line 88 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMA3gbjuP-v_SleBdG:-KcMA3gbjuP-v_SleBdH:be3r90u) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L88)):*\n> ```C\n>     assert(r.reply_type==AMQP_RESPONSE_NORMAL);\n> \n>     amqp_channel_open(conn, 1);\n> ```\n\nUse a variable for the channel number, and pass it into to other functions that require a channel.\n\n---\n\n*[examples/new_task.c, line 101 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMAGwbkwmAPWn16qP6:-KcMAGwbkwmAPWn16qP7:bk20g4l) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L101)):*\n> ```C\n>                                                          1,\n>                                                          amqp_cstring_bytes(queueName),\n>                                                          //  amqp_empty_bytes,\n> ```\n\nWhats commented out here?\n\n---\n\n*[examples/new_task.c, line 102 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMAR-ZZQfS46P-53Cc:-KcMAR-ZZQfS46P-53Cd:bgw3s2r) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L102)):*\n> ```C\n>                                                          amqp_cstring_bytes(queueName),\n>                                                          //  amqp_empty_bytes,\n>                                                          0,\n> ```\n\nAdd comment indicating what this means\n\n---\n\n*[examples/new_task.c, line 103 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMAMizecnGXsxSpiDv:-KcMAMizecnGXsxSpiDw:b-a8rkpw) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L103)):*\n> ```C\n>                                                          //  amqp_empty_bytes,\n>                                                          0,\n>                                                          1,// durable\n> ```\n\nUse the old-school /* */ comments (here and throughout).\n\n---\n\n*[examples/new_task.c, line 104 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcMAUZVBTUaZtXF0Yk-:-KcMAUZVBTUaZtXF0Yk0:b-j6246q) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L104)):*\n> ```C\n>                                                          0,\n>                                                          1,// durable\n>                                                          0,0,\n> ```\n\nAdd comments indicating what each of these mean (here and throughout - (this is an unfortunate failing of the library)).\n\n---\n\n*[examples/new_task.c, line 107 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM8QvT5jsa8mxMhExj:-KcM8QvUi3URdLH4G8ud:b-m5n66h) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L107)):*\n> ```C\n>                                                          amqp_empty_table);\n> \n>         r = amqp_get_rpc_reply(conn);\n> ```\n\nSame deal as with `amqp_channel_open()` above.  `amqp_queue_declare` has failed if `rq` is NULL, use `amqp_get_rpc_reply` to get details about the error under this condition.\n\n---\n\n*[examples/new_task.c, line 133 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM8wuiyR3ZAiJW1t0K:-KcM8wuiyR3ZAiJW1t0L:bgpz209) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L133)):*\n> ```C\n>     }\n> \n>     r=amqp_channel_close(conn, 1, AMQP_REPLY_SUCCESS);\n> ```\n\nNo need to do this, channels are implicitly closed when the connection is closed.\n\n---\n\n*[examples/new_task.c, line 135 at r1](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399#-KcM91-P3X1XNohO3rOM:-KcM91-P3X1XNohO3rON:b-um9gz6) ([raw file](https://github.com/alanxz/rabbitmq-c/blob/cbaa6478481259a2ab41f3a47967e3baf96c0d74/examples/new_task.c#L135)):*\n> ```C\n>     r=amqp_channel_close(conn, 1, AMQP_REPLY_SUCCESS);\n> \n>     r=amqp_connection_close(conn, AMQP_REPLY_SUCCESS); assert(r.reply_type==AMQP_RESPONSE_NORMAL);\n> ```\n\nUse a non-assert() error checking\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/alanxz/rabbitmq-c/399)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"alanxz"}}