)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"d83c37da2f27b08e3a184fff8c1589e42c64a1e8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TEMPEST_PLUGINS contains the list of the tempest plugins installed"},{"line_number":10,"context_line":"alongside tempest by lib/tempest."},{"line_number":11,"context_line":"If TEMPEST_PLUGINS is not explicitly set, the new devstack_tempest_plugins"},{"line_number":12,"context_line":"variable is used to fill it by combining its item (it is a list) with"},{"line_number":13,"context_line":"the base devstack path.."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_66dc34e4","line":11,"updated":"2019-03-12 11:03:34.000000000","message":"I don\u0027t like the design of simply ignoring the new variable if TEMPEST_PLUGINS is being set elsewhere, I think that this will lead to unexpected errors that are difficult to find.\n\nWhy would failing with an error in the event of a conflict be bad? At least generating a warning would seem needed to me.","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"7389d6d7cddfa2247f288801194e99c8d86e6d5f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TEMPEST_PLUGINS contains the list of the tempest plugins installed"},{"line_number":10,"context_line":"alongside tempest by lib/tempest."},{"line_number":11,"context_line":"If TEMPEST_PLUGINS is not explicitly set, the new devstack_tempest_plugins"},{"line_number":12,"context_line":"variable is used to fill it by combining its item (it is a list) with"},{"line_number":13,"context_line":"the base devstack path.."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_0601ab05","line":11,"in_reply_to":"5fc1f717_112fac8e","updated":"2019-03-12 14:33:45.000000000","message":"I think LIBS_FROM_GIT is different because the default is constructed from other things in the environment and not a variable specifically dedicated to setting it.","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"00aaa023f5097b92653c82e27a9ff915895c79c5","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TEMPEST_PLUGINS contains the list of the tempest plugins installed"},{"line_number":10,"context_line":"alongside tempest by lib/tempest."},{"line_number":11,"context_line":"If TEMPEST_PLUGINS is not explicitly set, the new devstack_tempest_plugins"},{"line_number":12,"context_line":"variable is used to fill it by combining its item (it is a list) with"},{"line_number":13,"context_line":"the base devstack path.."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_112fac8e","line":11,"in_reply_to":"5fc1f717_66dc34e4","updated":"2019-03-12 11:20:25.000000000","message":"I followed the same pattern followed by LIBS_FROM_GIT, the other variable set there.\n\nI\u0027m open to change everything of course (I\u0027d still like to have those changes in before branching.)","commit_id":"c92042727f89577c050251587c3f974843a6c211"}],"roles/write-devstack-local-conf/README.rst":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"83e96ae1368e440e25315bd07c6fed96e474ef2f","unresolved":false,"context_lines":[{"line_number":89,"context_line":"   ``plugin_requires`` in the plugin\u0027s settings file), this role will"},{"line_number":90,"context_line":"   automatically emit ``enable_plugin`` lines in the correct order."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":".. zuul:rolevar:: devstack_tempest_plugins"},{"line_number":93,"context_line":"   :type: list"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"   A list of tempest plugins which are installed alongside tempest."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5fc1f717_91d5cee7","line":92,"range":{"start_line":92,"start_character":18,"end_line":92,"end_character":42},"updated":"2019-03-12 21:17:00.000000000","message":"name is little confusing like is it devstack and tempest plugin both or just tempest plugin. \n\nwe should name it just tempest_plugins","commit_id":"6ce39292a7b83ddad6262efd8661b1e5d9cd6aa3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"83e96ae1368e440e25315bd07c6fed96e474ef2f","unresolved":false,"context_lines":[{"line_number":92,"context_line":".. zuul:rolevar:: devstack_tempest_plugins"},{"line_number":93,"context_line":"   :type: list"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"   A list of tempest plugins which are installed alongside tempest."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"   The list of values will be combined with the base devstack directory"},{"line_number":98,"context_line":"   and used to populate the ``TEMPEST_PLUGINS`` variable."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5fc1f717_d154b655","line":95,"range":{"start_line":95,"start_character":0,"end_line":95,"end_character":67},"updated":"2019-03-12 21:17:00.000000000","message":"also we need to mention about warning here something like if you use this var then you need to make sure TEMPEST_PLUGINS is not in loaclrc otherwise this var value will be ignored.","commit_id":"6ce39292a7b83ddad6262efd8661b1e5d9cd6aa3"}],"roles/write-devstack-local-conf/library/devstack_local_conf.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"b3a1b05ba0a9cd9ab94a86f3bf36ec3736d27d26","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            for tempest_plugin in self.tempest_plugins:"},{"line_number":273,"context_line":"                tp_dirs.append(os.path.join(self.base_dir, tempest_plugin))"},{"line_number":274,"context_line":"            self.localrc.append(\u0027TEMPEST_PLUGINS\u003d\"{}\"\u0027.format("},{"line_number":275,"context_line":"                    \u0027 \u0027.join(tp_dirs)))"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"    def handle_localconf(self, localconf):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_63d76225","line":275,"updated":"2019-03-11 05:43:56.000000000","message":"ahh, tricky, yes it\u0027s space not \",\" as a separator","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"83e96ae1368e440e25315bd07c6fed96e474ef2f","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                self.localrc.append(\u0027TEMPEST_PLUGINS\u003d\"{}\"\u0027.format("},{"line_number":280,"context_line":"                        \u0027 \u0027.join(tp_dirs)))"},{"line_number":281,"context_line":"            else:"},{"line_number":282,"context_line":"                self.warnings.append(\u0027TEMPEST_PLUGINS already defined ({}),\u0027"},{"line_number":283,"context_line":"                                     \u0027requested value {} ignored\u0027.format("},{"line_number":284,"context_line":"                                         tp, self.tempest_plugins))"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def handle_localconf(self, localconf):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_b1400aa7","line":284,"range":{"start_line":282,"start_character":0,"end_line":284,"end_character":67},"updated":"2019-03-12 21:17:00.000000000","message":"we need to be very careful about if anyone using this new var then they know about this warning. we should add the same in doc also.","commit_id":"6ce39292a7b83ddad6262efd8661b1e5d9cd6aa3"}],"roles/write-devstack-local-conf/library/test.py":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"d83c37da2f27b08e3a184fff8c1589e42c64a1e8","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                 plugins\u003dplugins,"},{"line_number":52,"context_line":"                 base_dir\u003d\u0027./test\u0027,"},{"line_number":53,"context_line":"                 path\u003dos.path.join(self.tmpdir, \u0027test.local.conf\u0027))"},{"line_number":54,"context_line":"        lc \u003d LocalConf(p.get(\u0027localrc\u0027),"},{"line_number":55,"context_line":"                       p.get(\u0027local_conf\u0027),"},{"line_number":56,"context_line":"                       p.get(\u0027base_services\u0027),"},{"line_number":57,"context_line":"                       p.get(\u0027services\u0027),"},{"line_number":58,"context_line":"                       p.get(\u0027plugins\u0027),"},{"line_number":59,"context_line":"                       p.get(\u0027base_dir\u0027),"},{"line_number":60,"context_line":"                       p.get(\u0027projects\u0027),"},{"line_number":61,"context_line":"                       p.get(\u0027project\u0027),"},{"line_number":62,"context_line":"                       p.get(\u0027tempest_plugins\u0027))"},{"line_number":63,"context_line":"        lc.write(p[\u0027path\u0027])"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        plugins \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_86260015","line":62,"range":{"start_line":54,"start_character":8,"end_line":62,"end_character":48},"updated":"2019-03-12 11:03:34.000000000","message":"The amount of patching you need to do in order to accomodate the new parameter IMHO shows the bad design being used here (not your fault, you just happen to make this visible by adding yet another positional parameter).\n\nWe could either hide this issue in the tests by making this a helper function, or fix it properly and make all of these kwargs so that this block of ten lines can simply be\n\n  lc \u003d LocalConf(**p)\n\nFine to do that in a separate patch, though.","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"00aaa023f5097b92653c82e27a9ff915895c79c5","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                 plugins\u003dplugins,"},{"line_number":52,"context_line":"                 base_dir\u003d\u0027./test\u0027,"},{"line_number":53,"context_line":"                 path\u003dos.path.join(self.tmpdir, \u0027test.local.conf\u0027))"},{"line_number":54,"context_line":"        lc \u003d LocalConf(p.get(\u0027localrc\u0027),"},{"line_number":55,"context_line":"                       p.get(\u0027local_conf\u0027),"},{"line_number":56,"context_line":"                       p.get(\u0027base_services\u0027),"},{"line_number":57,"context_line":"                       p.get(\u0027services\u0027),"},{"line_number":58,"context_line":"                       p.get(\u0027plugins\u0027),"},{"line_number":59,"context_line":"                       p.get(\u0027base_dir\u0027),"},{"line_number":60,"context_line":"                       p.get(\u0027projects\u0027),"},{"line_number":61,"context_line":"                       p.get(\u0027project\u0027),"},{"line_number":62,"context_line":"                       p.get(\u0027tempest_plugins\u0027))"},{"line_number":63,"context_line":"        lc.write(p[\u0027path\u0027])"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        plugins \u003d []"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_f14ea06d","line":62,"range":{"start_line":54,"start_character":8,"end_line":62,"end_character":48},"in_reply_to":"5fc1f717_86260015","updated":"2019-03-12 11:20:25.000000000","message":"A different patch is probably better to make the change less confusing.","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"d83c37da2f27b08e3a184fff8c1589e42c64a1e8","unresolved":false,"context_lines":[{"line_number":297,"context_line":"                       p.get(\u0027tempest_plugins\u0027))"},{"line_number":298,"context_line":"        lc.write(p[\u0027path\u0027])"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        tp \u003d None"},{"line_number":301,"context_line":"        with open(p[\u0027path\u0027]) as f:"},{"line_number":302,"context_line":"            for line in f:"},{"line_number":303,"context_line":"                if line.startswith(\u0027TEMPEST_PLUGINS\u0027):"},{"line_number":304,"context_line":"                    found \u003d line.strip().split(\u0027\u003d\u0027)[1]"},{"line_number":305,"context_line":"                    self.assertIsNone(tp,"},{"line_number":306,"context_line":"                        \"TEMPEST_PLUGIN ({}) found again ({})\".format("},{"line_number":307,"context_line":"                            tp, found))"},{"line_number":308,"context_line":"                    tp \u003d found"},{"line_number":309,"context_line":"        self.assertEqual(\u0027someplugin\u0027, tp)"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_66d2f402","line":308,"range":{"start_line":300,"start_character":0,"end_line":308,"end_character":30},"updated":"2019-03-12 11:03:34.000000000","message":"I guess this should go into a helper function to avoid repetition, too.","commit_id":"c92042727f89577c050251587c3f974843a6c211"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"00aaa023f5097b92653c82e27a9ff915895c79c5","unresolved":false,"context_lines":[{"line_number":297,"context_line":"                       p.get(\u0027tempest_plugins\u0027))"},{"line_number":298,"context_line":"        lc.write(p[\u0027path\u0027])"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        tp \u003d None"},{"line_number":301,"context_line":"        with open(p[\u0027path\u0027]) as f:"},{"line_number":302,"context_line":"            for line in f:"},{"line_number":303,"context_line":"                if line.startswith(\u0027TEMPEST_PLUGINS\u0027):"},{"line_number":304,"context_line":"                    found \u003d line.strip().split(\u0027\u003d\u0027)[1]"},{"line_number":305,"context_line":"                    self.assertIsNone(tp,"},{"line_number":306,"context_line":"                        \"TEMPEST_PLUGIN ({}) found again ({})\".format("},{"line_number":307,"context_line":"                            tp, found))"},{"line_number":308,"context_line":"                    tp \u003d found"},{"line_number":309,"context_line":"        self.assertEqual(\u0027someplugin\u0027, tp)"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_3148a85a","line":308,"range":{"start_line":300,"start_character":0,"end_line":308,"end_character":30},"in_reply_to":"5fc1f717_66d2f402","updated":"2019-03-12 11:20:25.000000000","message":"Totally agreed.","commit_id":"c92042727f89577c050251587c3f974843a6c211"}]}