)]}'
{"manilaclient/v2/shell.py":[{"author":{"_account_id":15100,"name":"zhongjun","email":"jun.zhongjun2@gmail.com","username":"zhongjun2"},"change_message_id":"c7f6d7f21041ba714b65e3060f0e95b110939b3a","unresolved":false,"context_lines":[{"line_number":2129,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2130,"context_line":""},{"line_number":2131,"context_line":"    def _is_public(share_type):"},{"line_number":2132,"context_line":"        return \u0027public\u0027 if share_type.is_public else \u0027private\u0027"},{"line_number":2133,"context_line":""},{"line_number":2134,"context_line":"    def _is_default(share_type):"},{"line_number":2135,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_c940e036","line":2132,"updated":"2015-11-19 09:57:21.000000000","message":"In line 2019, It has a same function(is public). It is better to make this function to common for both.","commit_id":"d1adc60f69ddb7c366a4a6bbe5d58d5cd16c7e0a"},{"author":{"_account_id":15100,"name":"zhongjun","email":"jun.zhongjun2@gmail.com","username":"zhongjun2"},"change_message_id":"c7f6d7f21041ba714b65e3060f0e95b110939b3a","unresolved":false,"context_lines":[{"line_number":2131,"context_line":"    def _is_public(share_type):"},{"line_number":2132,"context_line":"        return \u0027public\u0027 if share_type.is_public else \u0027private\u0027"},{"line_number":2133,"context_line":""},{"line_number":2134,"context_line":"    def _is_default(share_type):"},{"line_number":2135,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2136,"context_line":"            return \u0027YES\u0027"},{"line_number":2137,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_49d7104b","line":2134,"updated":"2015-11-19 09:57:21.000000000","message":"same, In line 2012.","commit_id":"d1adc60f69ddb7c366a4a6bbe5d58d5cd16c7e0a"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"67396ea904c49dd57df664796d1b7efa106274b2","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"        return \"N/A\""},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"def _is_public(share_type):"},{"line_number":2011,"context_line":"    return \u0027public\u0027 if share_type.is_public else \u0027private\u0027"},{"line_number":2012,"context_line":""},{"line_number":2013,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_7e9fdd50","line":2010,"updated":"2015-11-30 14:55:23.000000000","message":"Since it becomes public func and related only to share types, need to rename it. For example:\n\n\"_is_share_type_public\".","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"3308116eaa3f7134268cda08fa626771dada98f5","unresolved":false,"context_lines":[{"line_number":2007,"context_line":"        return \"N/A\""},{"line_number":2008,"context_line":""},{"line_number":2009,"context_line":""},{"line_number":2010,"context_line":"def _is_public(share_type):"},{"line_number":2011,"context_line":"    return \u0027public\u0027 if share_type.is_public else \u0027private\u0027"},{"line_number":2012,"context_line":""},{"line_number":2013,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_772fdff6","line":2010,"in_reply_to":"9a8ffd7b_7e9fdd50","updated":"2015-12-01 12:23:05.000000000","message":"Done","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"67396ea904c49dd57df664796d1b7efa106274b2","unresolved":false,"context_lines":[{"line_number":2127,"context_line":"    _print_share_type(stype)"},{"line_number":2128,"context_line":""},{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_5ef7798c","line":2130,"updated":"2015-11-30 14:55:23.000000000","message":"Would be better to place this function after \"_print_share_type_list\". Starting with line 2042.","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"3308116eaa3f7134268cda08fa626771dada98f5","unresolved":false,"context_lines":[{"line_number":2127,"context_line":"    _print_share_type(stype)"},{"line_number":2128,"context_line":""},{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_2a5c8eec","line":2130,"in_reply_to":"9a8ffd7b_5ef7798c","updated":"2015-12-01 12:23:05.000000000","message":"Done","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"67396ea904c49dd57df664796d1b7efa106274b2","unresolved":false,"context_lines":[{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2134,"context_line":"            return \u0027YES\u0027"},{"line_number":2135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_de1049ae","line":2132,"updated":"2015-11-30 14:55:23.000000000","message":"This one could be common one too, as you did for \"is_public\".","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"e84d7f718c33e252571ca40dd16b966fec6bb8a8","unresolved":false,"context_lines":[{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2134,"context_line":"            return \u0027YES\u0027"},{"line_number":2135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_c9710746","line":2132,"in_reply_to":"9a8ffd7b_0d1adb17","updated":"2015-12-02 07:32:52.000000000","message":"And i have made appropriate changes to functional test .. but i dun find what unit test can be added for this particular change ... as \n@ddt.unpack\n    def test_create(self, is_public, dhss, snapshot_support):\n        expected_body \u003d {\n            \"share_type\": {\n                \"name\": \u0027test-type-3\u0027,\n                \u0027os-share-type-access:is_public\u0027: is_public,\n                \"extra_specs\": {\n                    \"driver_handles_share_servers\": dhss,\n                    \"snapshot_support\": snapshot_support,\n                }\n            }\n        }\n\n        result \u003d cs.share_types.create(\n            \u0027test-type-3\u0027, dhss, snapshot_support, is_public\u003dis_public)\n        cs.assert_called(\u0027POST\u0027, \u0027/types\u0027, expected_body)\n        self.assertIsInstance(result, share_types.ShareType)\n\n\nThis unit test is already there which checks result of share type. Wht my change is in the client , representation of result ... do we need a unit test for this change .. ?","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"f397d769ffa6ef38a1b01bedba2c63f2d94c3638","unresolved":false,"context_lines":[{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2134,"context_line":"            return \u0027YES\u0027"},{"line_number":2135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_0d1adb17","line":2132,"in_reply_to":"9a8ffd7b_4b56a207","updated":"2015-12-01 17:16:48.000000000","message":"Ok, let it be so.","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"9dff849ed3ded3e3ba2d20374cdd703a62e572b0","unresolved":false,"context_lines":[{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2134,"context_line":"            return \u0027YES\u0027"},{"line_number":2135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_4e00ab66","line":2132,"in_reply_to":"9a8ffd7b_c9710746","updated":"2015-12-02 13:03:07.000000000","message":"Provided by you unit test uses \"python\" part. You changed CLI part, it is two different things. So, file that should contain new tests for your change is \"manilaclient/tests/unit/v2/test_shell.py\"","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"3308116eaa3f7134268cda08fa626771dada98f5","unresolved":false,"context_lines":[{"line_number":2129,"context_line":""},{"line_number":2130,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2131,"context_line":""},{"line_number":2132,"context_line":"    def _is_default(share_type):"},{"line_number":2133,"context_line":"        if share_type \u003d\u003d default_share_type:"},{"line_number":2134,"context_line":"            return \u0027YES\u0027"},{"line_number":2135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a8ffd7b_4b56a207","line":2132,"in_reply_to":"9a8ffd7b_de1049ae","updated":"2015-12-01 12:23:05.000000000","message":"HI Valeriy,\n\nI earlier tried same but could not do because ..\n\nIf i try to make it common \n--------------------------------------------------------------\ndef _is_share_type_default(share_type, default_share_type\u003dNone):\n    if share_type \u003d\u003d default_share_type:\n        return \u0027YES\u0027\n    else:\n        return \u0027-\u0027\n--------------------------------------------------------------\n\nIt requires the 2nd argument default_share_type t be passed to it \nwhere in ........\n\n _print_share_type_list(stypes, default_share_type\u003ddefault)\u003e\u003e\u003e\u003e\u003e\u003e\u003e2nd arg \n\n\n\n\ndef _print_share_type_list(stypes, default_share_type\u003dNone):\n\n    formatters \u003d {\n        \u0027Visibility\u0027: _is_share_type_public,\n        \u0027is_default\u0027: _is_share_type_default(\n                        default_share_type\u003ddefault_share_type),\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\n        \u0027required_extra_specs\u0027: _print_type_required_extra_specs,\n        \u0027optional_extra_specs\u0027: _print_type_optional_extra_specs,\n    }\n\n    for stype in stypes:\n        stype \u003d stype.to_dict()\n        stype[\u0027Visibility\u0027] \u003d stype.pop(\u0027is_public\u0027, \u0027unknown\u0027)\n\n    fields \u003d [\n        \u0027ID\u0027,\n        \u0027Name\u0027,\n        \u0027Visibility\u0027,\n        \u0027is_default\u0027,\n        \u0027required_extra_specs\u0027,\n        \u0027optional_extra_specs\u0027,\n    ]\n    cliutils.print_list(stypes, fields, formatters)\u003e\u003e\u003e\u003e\u003e\u003e\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nprint_list requires a callable function name in formatter, so will not be able to pass 2nd arg in that case ..\n\nbeing an inner function it takes the arg by default ...","commit_id":"30a75c06f2793e51c31ce26ab6bb9c21b0c1da8c"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"23ec65e29dd492717d95b517aa8b44460600758a","unresolved":false,"context_lines":[{"line_number":2041,"context_line":"    cliutils.print_list(stypes, fields, formatters)"},{"line_number":2042,"context_line":""},{"line_number":2043,"context_line":""},{"line_number":2044,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2045,"context_line":""},{"line_number":2046,"context_line":"    def _is_default(share_type):"},{"line_number":2047,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a8ffd7b_2e57675b","line":2044,"updated":"2015-12-02 13:05:05.000000000","message":"This is not covered with unit tests, BUT lots of existing functions too. So, would be great to see unit tests, but I do not insist.","commit_id":"959220b1f6f4195eaf289bc9baf02933343b19ab"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"0bfc4456ffff4a6c1e898732cae6956a9b5eb6a9","unresolved":false,"context_lines":[{"line_number":2041,"context_line":"    cliutils.print_list(stypes, fields, formatters)"},{"line_number":2042,"context_line":""},{"line_number":2043,"context_line":""},{"line_number":2044,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2045,"context_line":""},{"line_number":2046,"context_line":"    def _is_default(share_type):"},{"line_number":2047,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a8ffd7b_8552bb33","line":2044,"in_reply_to":"9a8ffd7b_2e57675b","updated":"2015-12-03 09:37:22.000000000","message":"For this fix https://review.openstack.org/#/c/245731/6/manilaclient/v2/shell.py\n-------------------------------------------------------------------------------------------\nThis is a test already written \n\n def test_create(self, is_public, dhss, snapshot_support):\n        expected_body \u003d {\n            \"share_type\": {\n                \"name\": \u0027test-type-3\u0027,\n                \u0027os-share-type-access:is_public\u0027: is_public,\n                \"extra_specs\": {\n                    \"driver_handles_share_servers\": dhss,\n                    \"snapshot_support\": snapshot_support,\n                }\n            }\n        }\n\n        result \u003d cs.share_types.create(\n            \u0027test-type-3\u0027, dhss, snapshot_support, is_public\u003dis_public)\n        cs.assert_called(\u0027POST\u0027, \u0027/types\u0027, expected_body)\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e it checks whether the api was called with expected parametrs.\n        self.assertIsInstance(result, share_types.ShareType)\n\n-----------------------------------------------------------------------------------------------------------------------------------\n\n\nwhat we are changing is just representation of the result obtained ....\n\n\ndef _print_share_type(stype, default_share_type\u003dNone):\n\n    def _is_default(share_type):\n        if share_type \u003d\u003d default_share_type:\n            return \u0027YES\u0027\n        else:\n            return \u0027-\u0027\n\n    stype_dict \u003d {\n        \u0027ID\u0027: stype.id,\n        \u0027Name\u0027: stype.name,\n        \u0027Visibility\u0027: _is_share_type_public(stype),\n        \u0027is_default\u0027: _is_default(stype),\n        \u0027required_extra_specs\u0027: _print_type_required_extra_specs(stype),\n        \u0027optional_extra_specs\u0027: _print_type_optional_extra_specs(stype),\n    }\n    cliutils.print_dict(stype_dict)\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003e\u003echanging how the result obtained from server would be displayed on cli\n\n\nIs it required to test this separately ? for understanding sake, please clarify .. \n\nActually i am seeing all unit tests written are making assert_called only none checking the result representation .. \n\nThere is something i am missing here to understand .. \nCan you see that ?","commit_id":"959220b1f6f4195eaf289bc9baf02933343b19ab"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"44e3bfe3e68d338e22c0b03456c002b44489907e","unresolved":false,"context_lines":[{"line_number":2041,"context_line":"    cliutils.print_list(stypes, fields, formatters)"},{"line_number":2042,"context_line":""},{"line_number":2043,"context_line":""},{"line_number":2044,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2045,"context_line":""},{"line_number":2046,"context_line":"    def _is_default(share_type):"},{"line_number":2047,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a8ffd7b_a676e0a1","line":2044,"in_reply_to":"9a8ffd7b_8552bb33","updated":"2015-12-03 10:45:58.000000000","message":"Test provided by you is related to \"python\" client of share types. It is different module. You changed \"shell\" module. So, test, you refer to has no relation to your changes at all.","commit_id":"959220b1f6f4195eaf289bc9baf02933343b19ab"},{"author":{"_account_id":16305,"name":"NidhiMittalHada","email":"nidhimittal19@gmail.com","username":"nidhimittalhada"},"change_message_id":"7511ab68455c678ba95d6ffad42053118a196c3f","unresolved":false,"context_lines":[{"line_number":2041,"context_line":"    cliutils.print_list(stypes, fields, formatters)"},{"line_number":2042,"context_line":""},{"line_number":2043,"context_line":""},{"line_number":2044,"context_line":"def _print_share_type(stype, default_share_type\u003dNone):"},{"line_number":2045,"context_line":""},{"line_number":2046,"context_line":"    def _is_default(share_type):"},{"line_number":2047,"context_line":"        if share_type \u003d\u003d default_share_type:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a8ffd7b_3999aac4","line":2044,"in_reply_to":"9a8ffd7b_a676e0a1","updated":"2015-12-03 12:29:19.000000000","message":"Done, As discussed this test is not required.","commit_id":"959220b1f6f4195eaf289bc9baf02933343b19ab"}]}