)]}'
{"ironicclient/common/utils.py":[{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"f36dd36515ddeddc4e331ea029b9a1cc260ab81a","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    if args.sort_key is not None:"},{"line_number":117,"context_line":"        # Support using both heading and field name for sort_key"},{"line_number":118,"context_line":"        fields_map \u003d dict(zip(field_labels, fields))"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            sort_key \u003d fields_map[args.sort_key]"},{"line_number":121,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_e7e09298","line":118,"updated":"2014-09-02 06:14:18.000000000","message":"i think we are trying to check if args.sort_key is in either field_lables or fields - why don\u0027t check for its existence in field_labels and fields list directly ? why do we need to construct a dict and check ?","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"418365fb8dc27c062088997e1d580d59900f083e","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    if args.sort_key is not None:"},{"line_number":117,"context_line":"        # Support using both heading and field name for sort_key"},{"line_number":118,"context_line":"        fields_map \u003d dict(zip(field_labels, fields))"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            sort_key \u003d fields_map[args.sort_key]"},{"line_number":121,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_7578bec4","line":118,"in_reply_to":"fa98f980_35e9d6b8","updated":"2014-09-02 11:32:00.000000000","message":"Possibly, but not really better is to me","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"3ea1244af9d55447c252fce0dbb1f8c3bfeb388b","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    if args.sort_key is not None:"},{"line_number":117,"context_line":"        # Support using both heading and field name for sort_key"},{"line_number":118,"context_line":"        fields_map \u003d dict(zip(field_labels, fields))"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            sort_key \u003d fields_map[args.sort_key]"},{"line_number":121,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_35e9d6b8","line":118,"in_reply_to":"fa98f980_75b0be43","updated":"2014-09-02 11:26:08.000000000","message":"just thought there was no need to create a new dict when the need was just to get the corresponding value of 1 element.  we could do like this as well:\n\ntry:\n    index \u003d field_labels.index(args.sort_key)\n    sort_key \u003d fields[index]\nexcept ValueError:\n    if args.sort_key in fields:\n        sort_key \u003d args.sort_key\n    else:\n        raise exc.CommandError","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43023d7ee281f15d708543c8b21709e16a7824af","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    if args.sort_key is not None:"},{"line_number":117,"context_line":"        # Support using both heading and field name for sort_key"},{"line_number":118,"context_line":"        fields_map \u003d dict(zip(field_labels, fields))"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            sort_key \u003d fields_map[args.sort_key]"},{"line_number":121,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_75b0be43","line":118,"in_reply_to":"fa98f980_e7e09298","updated":"2014-09-02 11:11:34.000000000","message":"because we also need to get associated value, not only check for existence","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":9315,"name":"ramesh","email":"rameshg87@gmail.com","username":"rameshg87"},"change_message_id":"f36dd36515ddeddc4e331ea029b9a1cc260ab81a","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                sort_key \u003d args.sort_key"},{"line_number":124,"context_line":"            else:"},{"line_number":125,"context_line":"                raise exc.CommandError("},{"line_number":126,"context_line":"                    _(\"%s is not a valid field for sorting\") %"},{"line_number":127,"context_line":"                    args.sort_key)"},{"line_number":128,"context_line":"        params[\u0027sort_key\u0027] \u003d sort_key"},{"line_number":129,"context_line":"    if args.sort_dir is not None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_6d8723d8","line":126,"updated":"2014-09-02 06:14:18.000000000","message":"does it make sense to print the valid values for sort_key here ?","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"43023d7ee281f15d708543c8b21709e16a7824af","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                sort_key \u003d args.sort_key"},{"line_number":124,"context_line":"            else:"},{"line_number":125,"context_line":"                raise exc.CommandError("},{"line_number":126,"context_line":"                    _(\"%s is not a valid field for sorting\") %"},{"line_number":127,"context_line":"                    args.sort_key)"},{"line_number":128,"context_line":"        params[\u0027sort_key\u0027] \u003d sort_key"},{"line_number":129,"context_line":"    if args.sort_dir is not None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_15877262","line":126,"in_reply_to":"fa98f980_6d8723d8","updated":"2014-09-02 11:11:34.000000000","message":"Done","commit_id":"757a9534bf74ccefa12189055709740b691a524d"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    return patch"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"def common_params_for_list(args, fields, field_labels):"},{"line_number":110,"context_line":"    params \u003d {}"},{"line_number":111,"context_line":"    if args.marker is not None:"},{"line_number":112,"context_line":"        params[\u0027marker\u0027] \u003d args.marker"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_c6a7c94e","line":109,"updated":"2014-09-11 20:11:18.000000000","message":"it would be nice to have a docstring for this","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":111,"context_line":"    if args.marker is not None:"},{"line_number":112,"context_line":"        params[\u0027marker\u0027] \u003d args.marker"},{"line_number":113,"context_line":"    if args.limit is not None:"},{"line_number":114,"context_line":"        params[\u0027limit\u0027] \u003d args.limit"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    if args.sort_key is not None:"},{"line_number":117,"context_line":"        # Support using both heading and field name for sort_key"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_01c9bb25","line":114,"updated":"2014-09-11 20:11:18.000000000","message":"Probably outside the scope of this patch, but I think it makes sense to check the limit value here instead of in common_filters, and raise an exc.CommandError if the value is non-integer or \u003c 0.","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        except KeyError:"},{"line_number":123,"context_line":"            raise exc.CommandError("},{"line_number":124,"context_line":"                _(\"%(sort_key)s is not a valid field for sorting, \""},{"line_number":125,"context_line":"                  \"valid are %(valid)s\") %"},{"line_number":126,"context_line":"                {\u0027sort_key\u0027: args.sort_key,"},{"line_number":127,"context_line":"                 \u0027valid\u0027: list(fields_map)})"},{"line_number":128,"context_line":"        params[\u0027sort_key\u0027] \u003d sort_key"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_e1e1172b","line":125,"updated":"2014-09-11 20:11:18.000000000","message":"I would suggest using \u0027sort-key\u0027 in the msg, since that\u0027s the name of the argument.\n\nEg \u0027\"%(sort_key)s\" is an invalid field for sorting. Valid values for --sort_key are: %(valid)s.\")","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        if args.sort_dir not in (\u0027asc\u0027, \u0027desc\u0027):"},{"line_number":131,"context_line":"            raise exc.CommandError("},{"line_number":132,"context_line":"                _(\"%s is not valid value for sort direction, \""},{"line_number":133,"context_line":"                  \"valid are \u0027asc\u0027 and \u0027desc\u0027\") %"},{"line_number":134,"context_line":"                args.sort_dir)"},{"line_number":135,"context_line":"        params[\u0027sort_dir\u0027] \u003d args.sort_dir"},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_61b7e7f5","line":133,"updated":"2014-09-11 20:11:18.000000000","message":"similar thing here.\n\n\"\u0027%s\u0027 is an invalid value for sort direction. Valid values for --sort-dir are: [\u0027asc\u0027, \u0027desc\u0027]\".\n\nsort-key values is a list of strings, so it may make sense to use a similar format for the sort-dir values too.","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    return params"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"def common_filters(marker, limit, sort_key, sort_dir):"},{"line_number":141,"context_line":"    filters \u003d []"},{"line_number":142,"context_line":"    if isinstance(limit, int) and limit \u003e 0:"},{"line_number":143,"context_line":"        filters.append(\u0027limit\u003d%s\u0027 % limit)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_6670dddc","line":140,"updated":"2014-09-11 20:11:18.000000000","message":"it would be nice to have a docstring for this","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"}],"ironicclient/tests/test_utils.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"75f09dc326e706e9137f6326971b4b886cd8ac45","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                          utils.common_params_for_list,"},{"line_number":113,"context_line":"                          self.args,"},{"line_number":114,"context_line":"                          [],"},{"line_number":115,"context_line":"                          [])"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_df7a39f0","line":115,"updated":"2014-09-11 15:57:03.000000000","message":"No tests for common_filters() I undestand that it\u0027s pretty straight forward, but it would be nice just to guarantee we have no regression if someone modifies that function","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"}],"ironicclient/v1/chassis_shell.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"72fa54419ae35d97fe27cd55c111530d299930f9","unresolved":false,"context_lines":[{"line_number":48,"context_line":"@cliutils.arg("},{"line_number":49,"context_line":"    \u0027--sort-key\u0027,"},{"line_number":50,"context_line":"    metavar\u003d\u0027\u003csort_key\u003e\u0027,"},{"line_number":51,"context_line":"    help\u003d\u0027Chassis field that will be used for sorting.\u0027)"},{"line_number":52,"context_line":"@cliutils.arg("},{"line_number":53,"context_line":"    \u0027--sort-dir\u0027,"},{"line_number":54,"context_line":"    metavar\u003d\u0027\u003csort_dir\u003e\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa98f980_81968b4a","line":51,"updated":"2014-09-11 20:11:18.000000000","message":"Default is to sort on UUID, right? (Is it worth mentioning?)\n\nhow easy would it be to provide a list of possible values? Just wondering, don\u0027t do it.","commit_id":"b416865ba3e9e6b2f985b9234010a244a8450b04"}]}