)]}'
{"swift/cli/ringbuilder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a9bf88e9451471fd415733e992ac8e5897dc44df","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    return parsed_devs"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"def devs_check(devs, str_1, str_2):"},{"line_number":186,"context_line":"    if not devs:"},{"line_number":187,"context_line":"        print(\"Search value matched 0 devices.\\n\""},{"line_number":188,"context_line":"              \"The on-disk ring builder is unchanged.\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a5de9d1_fe6725e8","line":185,"updated":"2016-02-02 19:40:22.000000000","message":"needs a doc string, and again - variable names are crap\n\n\"action_verb\" and \"aborting_noun\"\n\nI think the function should have \"confirm\" in the name somewhere too.\n\nWhen you do a method extraction like this it\u0027s *great* because it has the potential to reduce cognitive load.  Once you know what \"confirm_dev_action\" does you can skip over it anywhere you read it because you know what that call does.  But in order for that initial mental hook of \"what this line does\" to make it\u0027s way into your mind the function has to have have a clear documented contract.  A docstring can make all the difference there.  Once I believe the code in the function fulfills the contract described in the docstring I\u0027m all set!  I\u0027ll not only be able to read and reason about it better I\u0027ll remember to use it if possible rather than duplicating parts of it\u0027s functionality inline!","commit_id":"6f6636046b9ef47ee0307ae8760495a5d9f1e10a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a9bf88e9451471fd415733e992ac8e5897dc44df","unresolved":false,"context_lines":[{"line_number":249,"context_line":"def _set_info_values(devs, change):"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    if devs_check(devs, \u0027update the info for\u0027, \u0027modifications\u0027):"},{"line_number":252,"context_line":"        exit(EXIT_ERROR)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    for dev in devs:"},{"line_number":255,"context_line":"        orig_dev_string \u003d format_device(dev)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a5de9d1_fe940545","line":252,"updated":"2016-02-02 19:40:22.000000000","message":"should the exit get pulled into the method extract too?  Only reason I can think to leave it out is because it might make it easier to unittest\n\nself.assertFalse() instead of self.assertRaises(SystemExit)\n\n^ honestly not sure it\u0027s much different, the harder part will be mocking stdin, or input to simulate the user input...\n\n... but you don\u0027t add any new tests of the new method, so I don\u0027t know why not extract the exit too?","commit_id":"6f6636046b9ef47ee0307ae8760495a5d9f1e10a"}],"swift/common/direct_client.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c749533b08ca05e853a29ac973e60b9a0558aa6e","unresolved":false,"context_lines":[{"line_number":342,"context_line":"    if resp_chunk_size:"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        def _object_body():"},{"line_number":345,"context_line":"            buf \u003d resp.read(resp_chunk_size)"},{"line_number":346,"context_line":"            while buf:"},{"line_number":347,"context_line":"                yield buf"},{"line_number":348,"context_line":"                buf \u003d resp.read(resp_chunk_size)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_013026e7","side":"PARENT","line":345,"updated":"2015-12-07 18:21:39.000000000","message":"I don\u0027t think _get_resp works here becase it will have already read the body and thrown it away?","commit_id":"b3d6fa13195d09e317525f37dcab4ad6cf5f12c7"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c749533b08ca05e853a29ac973e60b9a0558aa6e","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"def _get_resp(conn_timeout, node, part, method, path, _header,"},{"line_number":53,"context_line":"              response_timeout, stype):"},{"line_number":54,"context_line":"    \"\"\"Return http response\"\"\""},{"line_number":55,"context_line":"    with Timeout(conn_timeout):"},{"line_number":56,"context_line":"        conn \u003d http_connect(node[\u0027ip\u0027], node[\u0027port\u0027], node[\u0027device\u0027], part,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_617a4a79","line":53,"updated":"2015-12-07 18:21:39.000000000","message":"this method would look better with a docstring that describes these params.\n\nI think it would be more intuitive if the call signature grouped required args and defaultable args in closer together:\n\n   def _get_resp(stype, node, part, method, path, headers\u003dNone,\n                 conn_timeout\u003d5, response_timeout\u003d15):\n       \"\"\"\n       Make request to backend storage node.\n\n       :param stype: a string, describing the type of service\n                     (i.e. \u0027Account\u0027, \u0027Container\u0027, \u0027Object\u0027)\n       :param node: a node dict from a ring\n       :param part: an integer, the partion number\n       :param method: a string, the HTTP method (e.g. \u0027PUT\u0027, \u0027DELETE\u0027, etc)\n       :param path: a string, the request path\n       :param headers: a dict, header name \u003d\u003e value\n       :param conn_timeout: timeout for connection\n       :param response_timeout: timeout for response\n\n       :returns: an HTTPResponse object\n       \"\"\"","commit_id":"23813bf88e0ce34cb237583259d48f11f88e5658"},{"author":{"_account_id":16896,"name":"Ondřej Nový","email":"novy@ondrej.org","username":"onovy"},"change_message_id":"e3ebf26d47cb97c53c21f0e65204a8630fc25041","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            http_reason\u003dresp.reason, http_headers\u003dheaders)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"def _get_resp(node, part, method, path, headers, stype,"},{"line_number":53,"context_line":"              conn_timeout\u003d5, response_timeout\u003d15):"},{"line_number":54,"context_line":"    \"\"\""},{"line_number":55,"context_line":"    Make request to backend storage node."}],"source_content_type":"text/x-python","patch_set":3,"id":"ba63e191_22e70242","line":52,"updated":"2016-01-18 18:33:43.000000000","message":"is _get_resp correct name of this func? exactly it\u0027s doing request.\n\nwhat about _make_req?","commit_id":"b8a6ee0888fcd1d0d8a710853f37ed9c4ab619cc"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"ca35ba869bf8954d403f9516a6e9be4caa91e2e8","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        headers \u003d {}"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    path \u003d \u0027/%s/%s/%s\u0027 % (account, container, obj)"},{"line_number":311,"context_line":"    resp \u003d _make_req(node, part, \u0027GET\u0027, path, gen_headers(headers), \u0027Object\u0027)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    if resp_chunk_size:"},{"line_number":314,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba63e191_be81c6fd","line":311,"updated":"2016-01-18 21:26:22.000000000","message":"the read on the resp shouldn\u0027t be called in this case unless there\u0027s an error.\n\nCertainly it\u0027s not correct to ignore the response from read in this case or some of the object body will be ignored!\n\nHow are the unittests still not catching this... grrr.","commit_id":"8beb5d2995e2fcc79e1217af72afd2e163bf40ae"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a9bf88e9451471fd415733e992ac8e5897dc44df","unresolved":false,"context_lines":[{"line_number":309,"context_line":"    path \u003d \u0027/%s/%s/%s\u0027 % (account, container, obj)"},{"line_number":310,"context_line":"    with Timeout(conn_timeout):"},{"line_number":311,"context_line":"        conn \u003d http_connect(node[\u0027ip\u0027], node[\u0027port\u0027], node[\u0027device\u0027], part,"},{"line_number":312,"context_line":"                            \u0027GET\u0027, path, headers\u003dgen_headers(headers))"},{"line_number":313,"context_line":"    with Timeout(response_timeout):"},{"line_number":314,"context_line":"        resp \u003d conn.getresponse()"},{"line_number":315,"context_line":"    if not is_success(resp.status):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a5de9d1_b39246f5","line":312,"updated":"2016-02-02 19:40:22.000000000","message":"ah, so you just don\u0027t use the new _make_req method with get_object at all - i\u0027m cool with that.","commit_id":"6f6636046b9ef47ee0307ae8760495a5d9f1e10a"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"cbb1e499fece9305672d59ba8acd8ca29f24c57e","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    path \u003d \u0027/%s\u0027 % account"},{"line_number":154,"context_line":"    _make_req(node, part, \u0027DELETE\u0027, path,"},{"line_number":155,"context_line":"              gen_headers(headers, True), \u0027Account\u0027)"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"def direct_head_container(node, part, account, container, conn_timeout\u003d5,"}],"source_content_type":"text/x-python","patch_set":11,"id":"dae33548_8f6a8fb1","line":155,"updated":"2016-02-17 19:23:13.000000000","message":"why isn\u0027t conn_timeout and response_timeout being passed to _make_req? if it\u0027s not needed, shouldn\u0027t it be removed?\n\nI\u0027m probably missing here because I noticed this was changed in patchset 5, but it is not obvious to me.\n\nsame for the other methods, so I won\u0027t repeat everywhere...","commit_id":"92558e13fe79d9759358b2d5db52dbe6ac49d179"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"82a8a339dcc511da55ad0920b4c3d838a884658b","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    path \u003d \u0027/%s\u0027 % account"},{"line_number":154,"context_line":"    _make_req(node, part, \u0027DELETE\u0027, path,"},{"line_number":155,"context_line":"              gen_headers(headers, True), \u0027Account\u0027)"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"def direct_head_container(node, part, account, container, conn_timeout\u003d5,"}],"source_content_type":"text/x-python","patch_set":11,"id":"dae33548_2aa891da","line":155,"in_reply_to":"dae33548_8f6a8fb1","updated":"2016-02-17 19:44:14.000000000","message":"oh good catch!\n\nYeah all the callers of _make_req should pass through the conn_timeout and response_timeout kwargs to _make_req!","commit_id":"92558e13fe79d9759358b2d5db52dbe6ac49d179"}],"swift/obj/ssync_sender.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"a9bf88e9451471fd415733e992ac8e5897dc44df","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            data +\u003d \u0027\\n\u0027"},{"line_number":249,"context_line":"        return data"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def line_check(self, str_1, str_2):"},{"line_number":252,"context_line":"        while True:"},{"line_number":253,"context_line":"            with exceptions.MessageTimeout(self.daemon.http_timeout, str_1):"},{"line_number":254,"context_line":"                line \u003d self.readline()"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a5de9d1_13747acd","line":251,"updated":"2016-02-02 19:40:22.000000000","message":"this would be worlds better with a doc string\n\nstr_1 and str_2 are probably nearly the worst possible variable names.\n\nmaybe \"description\" and \"expected_line\"","commit_id":"6f6636046b9ef47ee0307ae8760495a5d9f1e10a"}]}