)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"acc52a25664febdf126b8f4e921996acbc1dedef","unresolved":false,"context_lines":[{"line_number":19,"context_line":"Changes with cross-repo circular dependencies are required to share the"},{"line_number":20,"context_line":"same change queue."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Spec: https://review.opendev.org/#/c/643309/"},{"line_number":23,"context_line":"Change-Id: Ic121b2d8d057a7dc4448ae70045853347f265c6c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3fa7e38b_8e34e64b","line":22,"updated":"2019-10-09 10:25:52.000000000","message":"Could you add a depends-on to the spec to prevent it from accidental merge before the spec?","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9a6ea399f6a75dafd5150228ac5208c67bcbf144","unresolved":false,"context_lines":[{"line_number":19,"context_line":"Changes with cross-repo circular dependencies are required to share the"},{"line_number":20,"context_line":"same change queue."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Spec: https://review.opendev.org/#/c/643309/"},{"line_number":23,"context_line":"Change-Id: Ic121b2d8d057a7dc4448ae70045853347f265c6c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3fa7e38b_2e54d220","line":22,"in_reply_to":"3fa7e38b_8e34e64b","updated":"2019-10-09 11:13:45.000000000","message":"Done","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"}],"doc/source/admin/tenants.rst":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"acc52a25664febdf126b8f4e921996acbc1dedef","unresolved":false,"context_lines":[{"line_number":343,"context_line":"      are part of a dependency cycle."},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"      In case Zuul detects a dependency cycle it will make sure that every"},{"line_number":346,"context_line":"      change also include all other changes that are part of the cycle. However"},{"line_number":347,"context_line":"      each change will still be a normal item in the queue with its own jobs."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"      Changes with cross-repo circular dependencies are required to share the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_ae37624d","line":346,"updated":"2019-10-09 10:25:52.000000000","message":"nit: includes","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9a6ea399f6a75dafd5150228ac5208c67bcbf144","unresolved":false,"context_lines":[{"line_number":343,"context_line":"      are part of a dependency cycle."},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"      In case Zuul detects a dependency cycle it will make sure that every"},{"line_number":346,"context_line":"      change also include all other changes that are part of the cycle. However"},{"line_number":347,"context_line":"      each change will still be a normal item in the queue with its own jobs."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"      Changes with cross-repo circular dependencies are required to share the"}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_6e5aca50","line":346,"in_reply_to":"3fa7e38b_ae37624d","updated":"2019-10-09 11:13:45.000000000","message":"Done","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"acc52a25664febdf126b8f4e921996acbc1dedef","unresolved":false,"context_lines":[{"line_number":351,"context_line":"      postponed until all items in the cycle succeeded. In case of a failure in"},{"line_number":352,"context_line":"      any of those items the whole cycle will be dequeued."},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"      An error message will be posted to all items of th cycle in case some"},{"line_number":355,"context_line":"      items fail to report (e.g. merge failure when some items were already"},{"line_number":356,"context_line":"      merged). In this case the target branch(es) might be in a broken state."},{"line_number":357,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_4e3eee29","line":354,"updated":"2019-10-09 10:25:52.000000000","message":"the","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9a6ea399f6a75dafd5150228ac5208c67bcbf144","unresolved":false,"context_lines":[{"line_number":351,"context_line":"      postponed until all items in the cycle succeeded. In case of a failure in"},{"line_number":352,"context_line":"      any of those items the whole cycle will be dequeued."},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"      An error message will be posted to all items of th cycle in case some"},{"line_number":355,"context_line":"      items fail to report (e.g. merge failure when some items were already"},{"line_number":356,"context_line":"      merged). In this case the target branch(es) might be in a broken state."},{"line_number":357,"context_line":""}],"source_content_type":"text/x-rst","patch_set":6,"id":"3fa7e38b_0e59d658","line":354,"in_reply_to":"3fa7e38b_4e3eee29","updated":"2019-10-09 11:13:45.000000000","message":"Done","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"}],"tests/fixtures/config/circular-dependencies/git/common-config/zuul.yaml":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"9219679af90815e895a6022165493d6e5a46ad3a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        status: failure"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"- pipeline:"},{"line_number":30,"context_line":"    name: check-unused"},{"line_number":31,"context_line":"    manager: independent"},{"line_number":32,"context_line":"    trigger:"},{"line_number":33,"context_line":"      gerrit:"}],"source_content_type":"text/x-yaml","patch_set":50,"id":"c3945348_4e1b03c5","line":30,"updated":"2021-03-01 16:02:17.000000000","message":"Ack, thanks!","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"38b9d38b6cf5160887bb891875cd3b48cd14409e","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        status: failure"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"- pipeline:"},{"line_number":30,"context_line":"    name: check-unused"},{"line_number":31,"context_line":"    manager: independent"},{"line_number":32,"context_line":"    trigger:"},{"line_number":33,"context_line":"      gerrit:"}],"source_content_type":"text/x-yaml","patch_set":50,"id":"c3ea5274_92b2ad1a","line":30,"updated":"2021-02-26 22:45:56.000000000","message":"Why is this pipeline here?  (And gate too?)\n\nThey really clutter up the test output.  I find it helpful to read the logs from test runs to understand what zuul is doing and make sure that it\u0027s behaving as expected (and not doing something that we won\u0027t catch just from test assertions).  But all the extra queue messages from these pipelines make it pretty hard to read.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"8beaca5f098461b86f1799e2c015c73c67bbe57f","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        status: failure"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"- pipeline:"},{"line_number":30,"context_line":"    name: check-unused"},{"line_number":31,"context_line":"    manager: independent"},{"line_number":32,"context_line":"    trigger:"},{"line_number":33,"context_line":"      gerrit:"}],"source_content_type":"text/x-yaml","patch_set":50,"id":"3769e919_42d8279d","line":30,"in_reply_to":"c3ea5274_92b2ad1a","updated":"2021-03-01 07:01:32.000000000","message":"I think the reason for adding those pipelines was to test a certain edge case. However, I don\u0027t remember the details anymore and I also can\u0027t find a related test case. Given that the tests are still passing w/o them in https://review.opendev.org/c/zuul/zuul/+/777843/ I think it makes sense to remove them.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"}],"zuul/configloader.py":[{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"131b9caf4aa8a524838f3c51cae8f162e06b3b8e","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"        queue \u003d model.Queue("},{"line_number":1385,"context_line":"            conf[\u0027name\u0027],"},{"line_number":1386,"context_line":"            conf.get(\u0027per-branch\u0027, False),"},{"line_number":1387,"context_line":"            conf.get(\u0027allow-circular-dependencies\u0027, False),"},{"line_number":1388,"context_line":"        )"},{"line_number":1389,"context_line":"        queue.source_context \u003d conf.get(\u0027_source_context\u0027)"},{"line_number":1390,"context_line":"        queue.start_mark \u003d conf.get(\u0027_start_mark\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"ff570b3c_3d7610da","line":1387,"updated":"2020-05-27 11:32:40.000000000","message":"Shouldn\u0027t this check that the project or tenant allow circular dependencies?","commit_id":"bdf483ff281a90852c8a336d2da1221e4b459773"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"96620ab0641d442ae55dd39df643e5dad868226f","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"        queue \u003d model.Queue("},{"line_number":1385,"context_line":"            conf[\u0027name\u0027],"},{"line_number":1386,"context_line":"            conf.get(\u0027per-branch\u0027, False),"},{"line_number":1387,"context_line":"            conf.get(\u0027allow-circular-dependencies\u0027, False),"},{"line_number":1388,"context_line":"        )"},{"line_number":1389,"context_line":"        queue.source_context \u003d conf.get(\u0027_source_context\u0027)"},{"line_number":1390,"context_line":"        queue.start_mark \u003d conf.get(\u0027_start_mark\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"ff570b3c_984f6a69","line":1387,"in_reply_to":"ff570b3c_3d7610da","updated":"2020-05-27 11:44:17.000000000","message":"Specifying this in the tenant config was done in the spec. However there was a discussion on IRC a few weeks back with the conclusion that it\u0027s actually better to decide this based on the queue since circular dependencies operate in the same shared queue which makes this a natural choice.\n\nThis way it\u0027s more flexible as the project can decide without an admin while still keeping the possibility to disallow cyclic deps in a config project.","commit_id":"bdf483ff281a90852c8a336d2da1221e4b459773"}],"zuul/manager/__init__.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"bf8a51d66d4d6d04d1dff20d248e2f9a28138fc2","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                             \"does not allow circular dependencies\", change)"},{"line_number":357,"context_line":"                    self.dequeueItem(item)"},{"line_number":358,"context_line":"                    actions \u003d self.pipeline.failure_actions"},{"line_number":359,"context_line":"                    ci \u003d model.QueueItem(self, cycle[-1], event)"},{"line_number":360,"context_line":"                    ci.warning(\"Dependency cycle detected\")"},{"line_number":361,"context_line":"                    ci.setReportedResult(\u0027FAILURE\u0027)"},{"line_number":362,"context_line":"                    self.sendReport(actions, ci)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_bb62d4ac","line":359,"updated":"2019-09-30 08:57:27.000000000","message":"Maybe it would be better to report an error to all items in the cycle?","commit_id":"08a3966879208fed0382b331d720fe5618762efe"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"acc52a25664febdf126b8f4e921996acbc1dedef","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            if hasattr(change, \"needs_changes\"):"},{"line_number":352,"context_line":"                cycle \u003d self.cycleForChange(change, dependency_graph, event)"},{"line_number":353,"context_line":"                if cycle and not self.canProcessCycle(cycle,"},{"line_number":354,"context_line":"                                                      item.pipeline.tenant):"},{"line_number":355,"context_line":"                    log.info(\"Dequeing change %s since at least one project \""},{"line_number":356,"context_line":"                             \"does not allow circular dependencies\", change)"},{"line_number":357,"context_line":"                    self.dequeueItem(item)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_6e396a3d","line":354,"updated":"2019-10-09 10:25:52.000000000","message":"Is it on purpose to check this after enqueue?","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9a6ea399f6a75dafd5150228ac5208c67bcbf144","unresolved":false,"context_lines":[{"line_number":351,"context_line":"            if hasattr(change, \"needs_changes\"):"},{"line_number":352,"context_line":"                cycle \u003d self.cycleForChange(change, dependency_graph, event)"},{"line_number":353,"context_line":"                if cycle and not self.canProcessCycle(cycle,"},{"line_number":354,"context_line":"                                                      item.pipeline.tenant):"},{"line_number":355,"context_line":"                    log.info(\"Dequeing change %s since at least one project \""},{"line_number":356,"context_line":"                             \"does not allow circular dependencies\", change)"},{"line_number":357,"context_line":"                    self.dequeueItem(item)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_b4cd70b8","line":354,"in_reply_to":"3fa7e38b_6e396a3d","updated":"2019-10-09 11:13:45.000000000","message":"No, I will move it up to check before enqueuing the change.","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"acc52a25664febdf126b8f4e921996acbc1dedef","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    def canProcessCycle(self, cycle, tenant):"},{"line_number":400,"context_line":"        projects \u003d ["},{"line_number":401,"context_line":"            c.project for c in cycle"},{"line_number":402,"context_line":"            if tenant.getProject(c.project.canonical_name)[1]"},{"line_number":403,"context_line":"        ]"},{"line_number":404,"context_line":"        return all(tenant.getAllowCircularDependencies(p) for p in projects)"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_0e48f6c9","line":402,"updated":"2019-10-09 10:25:52.000000000","message":"Are there cases where the if clause is false?","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"9a6ea399f6a75dafd5150228ac5208c67bcbf144","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    def canProcessCycle(self, cycle, tenant):"},{"line_number":400,"context_line":"        projects \u003d ["},{"line_number":401,"context_line":"            c.project for c in cycle"},{"line_number":402,"context_line":"            if tenant.getProject(c.project.canonical_name)[1]"},{"line_number":403,"context_line":"        ]"},{"line_number":404,"context_line":"        return all(tenant.getAllowCircularDependencies(p) for p in projects)"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_1451e4d9","line":402,"in_reply_to":"3fa7e38b_0e48f6c9","updated":"2019-10-09 11:13:45.000000000","message":"getProject() can return (None, None) in some cases, so I figured it might be better to check.\n\nThis is also how it\u0027s done in scheduleMerge().\n\nNot sure though if this can ever be None here.","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"2b58a0b8b18c51eacdd302c6ace61a8f8bb9e93a","unresolved":false,"context_lines":[{"line_number":399,"context_line":"    def canProcessCycle(self, cycle, tenant):"},{"line_number":400,"context_line":"        projects \u003d ["},{"line_number":401,"context_line":"            c.project for c in cycle"},{"line_number":402,"context_line":"            if tenant.getProject(c.project.canonical_name)[1]"},{"line_number":403,"context_line":"        ]"},{"line_number":404,"context_line":"        return all(tenant.getAllowCircularDependencies(p) for p in projects)"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_f4c1a855","line":402,"in_reply_to":"3fa7e38b_1451e4d9","updated":"2019-10-09 11:23:10.000000000","message":"k, I don\u0027t think it could be here because an item cannot depend on a change of a project that is not part of the tenant. But there were discussions about supporting foreign deps long time ago and in this case this check can fail. So it\u0027s probably better to keep it.","commit_id":"1771d4f724d05acf136d4f6701d94945e76a77f1"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"faf0662fc0eb318dea2f2414aa74e0d5003aa5ae","unresolved":false,"context_lines":[{"line_number":9,"context_line":"# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":"import abc"},{"line_number":13,"context_line":"import collections"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import textwrap"}],"source_content_type":"text/x-python","patch_set":22,"id":"1f493fa4_5ed0fbf7","line":12,"updated":"2020-05-08 16:28:57.000000000","message":"linters: F401 \u0027abc\u0027 imported but unused","commit_id":"18441eaeadd783f0874ddea7f513047803b59c9e"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"131b9caf4aa8a524838f3c51cae8f162e06b3b8e","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        log.debug(\"Considering adding change %s\" % change)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        history \u003d history if history is not None else []"},{"line_number":330,"context_line":"        log.debug(\"History: %s\", history)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        # Ensure the dependency graph is created when the first change is"},{"line_number":333,"context_line":"        # process to allow cycle detection with the Tarjan algorithm"}],"source_content_type":"text/x-python","patch_set":28,"id":"ff570b3c_bdefc085","line":330,"updated":"2020-05-27 11:32:40.000000000","message":"perhaps this new debug statement could be done only if the history or the dependency_graph contain something?","commit_id":"bdf483ff281a90852c8a336d2da1221e4b459773"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":410,"context_line":"            if not cycle:"},{"line_number":411,"context_line":"                # Only enqueue changes behind if this is not a cycle, as"},{"line_number":412,"context_line":"                # cycle changes will otherwise be partially enqueued"},{"line_number":413,"context_line":"                # without any error handling"},{"line_number":414,"context_line":"                self.enqueueChangesBehind(change, event, quiet,"},{"line_number":415,"context_line":"                                          ignore_requirements, change_queue,"},{"line_number":416,"context_line":"                                          history, dependency_graph)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_75d078f8","line":413,"updated":"2020-05-28 21:55:20.000000000","message":"I don\u0027t think I fully understand this.  If we have a cycle A-B, why can\u0027t we enqueue C that depends on A?  Final queue would be: (A, B), C.\n\nIf the issue is that when we get to this point in the code after enqueing A we don\u0027t want to enqueue B using this method, that makes sense, but we have knowledge of the cycle, so can we use that?  I think this is safe if we proceed only after the cycle is complete.  How about this process:\n\n* Add A, identify B as a dependency in enqueueChangesAhead, recurse into this for B\n  * Add B, identify A as a dependency already enqueued, no action needed\n  * Identify A-B as a cycle\n  * Enqueue changes behind B\n    * The full cycle has been enqueued, so we can safely enqueue changess behind it\n    * For each member of the cycle, enqueue changes behind it:\n      * (member\u003dA): B is behind A but is part of the cycle, so skip\n      * (member\u003dA): C is behind A and is not part of the cycle, so enqueue it\n      * (member\u003dB): A is behind B but is part of the cycle, so skip\n* Identify A-B as a cycle (we\u0027re back in this method for A)\n* Enqueue changes behind A\n  * We\u0027d run the algorithm again here, which isn\u0027t necessary but should be safe as it should no-op.  Maybe we can optimize by checking history.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":410,"context_line":"            if not cycle:"},{"line_number":411,"context_line":"                # Only enqueue changes behind if this is not a cycle, as"},{"line_number":412,"context_line":"                # cycle changes will otherwise be partially enqueued"},{"line_number":413,"context_line":"                # without any error handling"},{"line_number":414,"context_line":"                self.enqueueChangesBehind(change, event, quiet,"},{"line_number":415,"context_line":"                                          ignore_requirements, change_queue,"},{"line_number":416,"context_line":"                                          history, dependency_graph)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_e0401238","line":413,"in_reply_to":"ff570b3c_75d078f8","updated":"2020-06-02 12:50:09.000000000","message":"You are right. I think I did not really consider enqueuing changes behind with my Github centric world view ;)\n\nWill fix.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        jobs_to_cancel \u003d item.getJobs()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        if (prime and item.current_build_set.ref and not"},{"line_number":612,"context_line":"                item.didBundleStartReporting()):"},{"line_number":613,"context_line":"            item.resetAllBuilds()"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"        for job in jobs_to_cancel:"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_55cbb448","line":612,"updated":"2020-05-28 21:55:20.000000000","message":"I think this could use a comment about what happens in this case.  How do we get to a case where we have started reporting a bundle but are canceling jobs?  Why is it okay not to clear the builds here?  Is it because the only reason we would cancel builds after a bundle starts reporting is because the bundle has failed, and therefore we know it\u0027s going to be dequeued immediately afterwords?  If we don\u0027t change this code, does anything bad happen?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":609,"context_line":"        jobs_to_cancel \u003d item.getJobs()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        if (prime and item.current_build_set.ref and not"},{"line_number":612,"context_line":"                item.didBundleStartReporting()):"},{"line_number":613,"context_line":"            item.resetAllBuilds()"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"        for job in jobs_to_cancel:"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_750fa687","line":612,"in_reply_to":"ff570b3c_55cbb448","updated":"2020-06-02 12:50:09.000000000","message":"Yes, this is for the case where a bundle is failing.\n\nSince an item that is part of a bundle will also fail if any other item in the bundle failed we want to keep the build results. Otherwise the reporting might be misleading and/or incomplete.\n\nWill add a comment to make this clear.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        for job in jobs_to_cancel:"},{"line_number":616,"context_line":"            build \u003d old_build_set.getBuild(job.name)"},{"line_number":617,"context_line":"            if not build or not build.result:"},{"line_number":618,"context_line":"                self.sched.cancelJob(old_build_set, job)"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        for item_behind in item.items_behind:"},{"line_number":621,"context_line":"            log.debug(\"Canceling jobs for change %s, behind change %s\","}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_b5da1014","line":618,"updated":"2020-05-28 21:55:20.000000000","message":"Similar question here: why is this changing?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":615,"context_line":"        for job in jobs_to_cancel:"},{"line_number":616,"context_line":"            build \u003d old_build_set.getBuild(job.name)"},{"line_number":617,"context_line":"            if not build or not build.result:"},{"line_number":618,"context_line":"                self.sched.cancelJob(old_build_set, job)"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":"        for item_behind in item.items_behind:"},{"line_number":621,"context_line":"            log.debug(\"Canceling jobs for change %s, behind change %s\","}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_35688ef3","line":618,"in_reply_to":"ff570b3c_b5da1014","updated":"2020-06-02 12:50:09.000000000","message":"The reason for this change was to only cancel jobs that are still running. With the change in the scheduler\u0027s cancelJob() method this is no longer necessary. I\u0027ll remove it.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":768,"context_line":"            for bundle_item in item.bundle.items:"},{"line_number":769,"context_line":"                if bundle_item.change.updatesConfig("},{"line_number":770,"context_line":"                        bundle_item.pipeline.tenant):"},{"line_number":771,"context_line":"                    return True"},{"line_number":772,"context_line":"        while item:"},{"line_number":773,"context_line":"            if item.change.updatesConfig(item.pipeline.tenant):"},{"line_number":774,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_95d54ce8","line":771,"updated":"2020-05-28 21:55:20.000000000","message":"I had some questions about this (why doesn\u0027t the original code work?  what about bundles ahead of items?) but in trying to answer them, I can\u0027t actually find this used anywhere.  Is method dead code?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":768,"context_line":"            for bundle_item in item.bundle.items:"},{"line_number":769,"context_line":"                if bundle_item.change.updatesConfig("},{"line_number":770,"context_line":"                        bundle_item.pipeline.tenant):"},{"line_number":771,"context_line":"                    return True"},{"line_number":772,"context_line":"        while item:"},{"line_number":773,"context_line":"            if item.change.updatesConfig(item.pipeline.tenant):"},{"line_number":774,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_157d8ab1","line":771,"in_reply_to":"ff570b3c_95d54ce8","updated":"2020-06-02 12:50:09.000000000","message":"Yes, I think this is dead code. Will remove it.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":1298,"context_line":"            actions \u003d self.pipeline.merge_failure_actions"},{"line_number":1299,"context_line":"            item.setReportedResult(\u0027CONFIG_ERROR\u0027)"},{"line_number":1300,"context_line":"        elif item.didMergerFail():"},{"line_number":1301,"context_line":"            log.debug(\"merger failure\")"},{"line_number":1302,"context_line":"            actions \u003d self.pipeline.merge_failure_actions"},{"line_number":1303,"context_line":"            item.setReportedResult(\u0027MERGER_FAILURE\u0027)"},{"line_number":1304,"context_line":"        elif item.wasDequeuedNeedingChange():"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_f505a87b","line":1301,"updated":"2020-05-28 21:55:20.000000000","message":"nit: Maybe capitalize these if you want to keep them?  But maybe they aren\u0027t necessary?  I think the reporter log lines tell us this?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":1298,"context_line":"            actions \u003d self.pipeline.merge_failure_actions"},{"line_number":1299,"context_line":"            item.setReportedResult(\u0027CONFIG_ERROR\u0027)"},{"line_number":1300,"context_line":"        elif item.didMergerFail():"},{"line_number":1301,"context_line":"            log.debug(\"merger failure\")"},{"line_number":1302,"context_line":"            actions \u003d self.pipeline.merge_failure_actions"},{"line_number":1303,"context_line":"            item.setReportedResult(\u0027MERGER_FAILURE\u0027)"},{"line_number":1304,"context_line":"        elif item.wasDequeuedNeedingChange():"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_3a1fdd6c","line":1301,"in_reply_to":"ff570b3c_f505a87b","updated":"2020-06-02 12:50:09.000000000","message":"Will capitalize. I found those messages helpful for debugging, so I\u0027d like to keep them if there are no strong objections. IIRC there were other messages in here no capitalized and I wanted to be consistent. Unfortunately I can\u0027t find those messages anymore and now I look like a fool :P","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"9219679af90815e895a6022165493d6e5a46ad3a","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                                          history, dependency_graph)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                self.log.debug(\"Skip enqueueing changes behind because of \""},{"line_number":434,"context_line":"                               \"dependency cycle\")"},{"line_number":435,"context_line":"            zuul_driver \u003d self.sched.connections.drivers[\u0027zuul\u0027]"},{"line_number":436,"context_line":"            tenant \u003d self.pipeline.tenant"},{"line_number":437,"context_line":"            zuul_driver.onChangeEnqueued("}],"source_content_type":"text/x-python","patch_set":50,"id":"79b9bd39_afb1eb3c","line":434,"updated":"2021-03-01 16:02:17.000000000","message":"Ack.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"38b9d38b6cf5160887bb891875cd3b48cd14409e","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                                          history, dependency_graph)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                self.log.debug(\"Skip enqueueing changes behind because of \""},{"line_number":434,"context_line":"                               \"dependency cycle\")"},{"line_number":435,"context_line":"            zuul_driver \u003d self.sched.connections.drivers[\u0027zuul\u0027]"},{"line_number":436,"context_line":"            tenant \u003d self.pipeline.tenant"},{"line_number":437,"context_line":"            zuul_driver.onChangeEnqueued("}],"source_content_type":"text/x-python","patch_set":50,"id":"cf243867_006ba327","line":434,"updated":"2021-02-26 22:45:56.000000000","message":"On ps29 we agreed to support changes behind, but in ps 36 this was added with no followup comment on the review.  It would be helpful to reply to old comments if you change your mind.\n\nI have implemented the algorithm I suggested in ps29 and will upload it as a followup.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"8beaca5f098461b86f1799e2c015c73c67bbe57f","unresolved":false,"context_lines":[{"line_number":431,"context_line":"                                          history, dependency_graph)"},{"line_number":432,"context_line":"            else:"},{"line_number":433,"context_line":"                self.log.debug(\"Skip enqueueing changes behind because of \""},{"line_number":434,"context_line":"                               \"dependency cycle\")"},{"line_number":435,"context_line":"            zuul_driver \u003d self.sched.connections.drivers[\u0027zuul\u0027]"},{"line_number":436,"context_line":"            tenant \u003d self.pipeline.tenant"},{"line_number":437,"context_line":"            zuul_driver.onChangeEnqueued("}],"source_content_type":"text/x-python","patch_set":50,"id":"a0f9e175_b4d9e063","line":434,"in_reply_to":"cf243867_006ba327","updated":"2021-03-01 07:01:32.000000000","message":"Thanks for taking care of that. Looking at the diff between ps29 and ps30 I think I was under the impression that this could be solved w/o any additional logic (I was obviously wrong).","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"}],"zuul/model.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"9219679af90815e895a6022165493d6e5a46ad3a","unresolved":false,"context_lines":[{"line_number":4133,"context_line":"        if ("},{"line_number":4134,"context_line":"            md.queue_name is None"},{"line_number":4135,"context_line":"            and project_config.queue_name is not None"},{"line_number":4136,"context_line":"        ):"},{"line_number":4137,"context_line":"            md.queue_name \u003d project_config.queue_name"},{"line_number":4138,"context_line":""},{"line_number":4139,"context_line":"    def getProjectConfigs(self, name):"}],"source_content_type":"text/x-python","patch_set":50,"id":"8dbe36bf_e8d967e6","line":4136,"updated":"2021-03-01 16:02:17.000000000","message":"It looks like we disagree on this.  I will say that from my pov, it\u0027s important not to waste vertical space (I don\u0027t mean never add an extra line, vertical space can be really useful), but excess use of vertical space means I have to scroll more to read the code.\n\nThere is more than one way to write this in two lines.  Other Zuul contributors share your concern about scanning, and in Zuul they have chosen to adopt this practice:\n\nif (something\n        and something_else):\n   do_things()\n\nThe Zuul project has adopted pep8 as a style guide, and we do generally adhere to that (including the frequently ignored \"A Foolish Consistency is the Hobgoblin of Little Minds\" admonition; we\u0027re not super-strict).  Pep8 does discuss this issue in this section:\n\n  https://www.python.org/dev/peps/pep-0008/#indentation\n\nWhere it suggests several solutions (scan down to the 4th verbatim block).  The 4 line version is not one of them.\n\nPlease do consider adopting that form of the 2 line version.  It is used by your colleagues already, I think it may address your concerns while also mitigating mine, so it may be a good compromise.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"38b9d38b6cf5160887bb891875cd3b48cd14409e","unresolved":false,"context_lines":[{"line_number":4133,"context_line":"        if ("},{"line_number":4134,"context_line":"            md.queue_name is None"},{"line_number":4135,"context_line":"            and project_config.queue_name is not None"},{"line_number":4136,"context_line":"        ):"},{"line_number":4137,"context_line":"            md.queue_name \u003d project_config.queue_name"},{"line_number":4138,"context_line":""},{"line_number":4139,"context_line":"    def getProjectConfigs(self, name):"}],"source_content_type":"text/x-python","patch_set":50,"id":"56d0cef0_609e30e1","line":4136,"updated":"2021-02-26 22:45:56.000000000","message":"Nit: maybe consider a 2 line version of this?  Any of the ways to write this in 2 lines would more closely match the predominant style and conserve vertical space (if we did the whole method like this, it would grow from 15 to 26 lines).","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"281ec015ae9ba00acdc8a950b67f766d39b00564","unresolved":false,"context_lines":[{"line_number":4133,"context_line":"        if ("},{"line_number":4134,"context_line":"            md.queue_name is None"},{"line_number":4135,"context_line":"            and project_config.queue_name is not None"},{"line_number":4136,"context_line":"        ):"},{"line_number":4137,"context_line":"            md.queue_name \u003d project_config.queue_name"},{"line_number":4138,"context_line":""},{"line_number":4139,"context_line":"    def getProjectConfigs(self, name):"}],"source_content_type":"text/x-python","patch_set":50,"id":"d98e783d_22e6869d","line":4136,"in_reply_to":"56d0cef0_609e30e1","updated":"2021-03-01 06:43:11.000000000","message":"To be honest I find this formatting much more readable than the 2 line version as you can better differentiate between the if condition and the body of the statement. In the 2 line version you always have to search for the end of the if condition \"):\" to see where the body line starts. I\u0027d also find this method much more readable if it would take 26 instead of 15 lines and I don\u0027t see the point in saving lines of code in such cases.","commit_id":"50c266565e58acce2dd69e00734c7a79c2a56d80"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"03a89ea39199a23381dd8866a68b3357c528e181","unresolved":false,"context_lines":[{"line_number":2228,"context_line":""},{"line_number":2229,"context_line":"        # Additional container for connection specifig information to be used"},{"line_number":2230,"context_line":"        # by reporters throughout the lifecycle"},{"line_number":2231,"context_line":"        self.dynamic_state \u003d defaultdict(dict)"},{"line_number":2232,"context_line":""},{"line_number":2233,"context_line":"        # A bundle holds other queue items that have to be successful"},{"line_number":2234,"context_line":"        # for the current queue item to succeed"}],"source_content_type":"text/x-python","patch_set":51,"id":"12a906ef_d9483f3a","line":2231,"updated":"2021-03-01 18:50:00.000000000","message":"This is where the conflict was; I verified no code changes.","commit_id":"5161347efdfce1a3dac020e88b20b4f632565cd2"}],"zuul/reporter/__init__.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if item.dequeued_needing_change:"},{"line_number":179,"context_line":"            msg \u003d \u0027This change depends on a change that failed to merge.\\n\u0027"},{"line_number":180,"context_line":"        elif item.isBundleFailing():"},{"line_number":181,"context_line":"            msg \u003d \u0027This change is part of a bundle that is failing.\\n\u0027"},{"line_number":182,"context_line":"            if with_jobs:"},{"line_number":183,"context_line":"                msg \u003d \u0027{}\\n\\n{}\u0027.format(msg, self._formatItemReportJobs(item))"},{"line_number":184,"context_line":"            msg \u003d \"{}\\n\\n{}\".format("}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_d5086492","line":181,"updated":"2020-05-28 21:55:20.000000000","message":"Maybe use the past tense (\"that failed\")?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if item.dequeued_needing_change:"},{"line_number":179,"context_line":"            msg \u003d \u0027This change depends on a change that failed to merge.\\n\u0027"},{"line_number":180,"context_line":"        elif item.isBundleFailing():"},{"line_number":181,"context_line":"            msg \u003d \u0027This change is part of a bundle that is failing.\\n\u0027"},{"line_number":182,"context_line":"            if with_jobs:"},{"line_number":183,"context_line":"                msg \u003d \u0027{}\\n\\n{}\u0027.format(msg, self._formatItemReportJobs(item))"},{"line_number":184,"context_line":"            msg \u003d \"{}\\n\\n{}\".format("}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_35cc0e34","line":181,"in_reply_to":"ff570b3c_d5086492","updated":"2020-06-02 12:50:09.000000000","message":"Done","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"}],"zuul/scheduler.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"779a60458889d4437777d38b11e61fc56336433a","unresolved":false,"context_lines":[{"line_number":1734,"context_line":"                        self.nodepool.returnNodeSet("},{"line_number":1735,"context_line":"                            nodeset, build\u003dbuild, zuul_event_id\u003ditem.event)"},{"line_number":1736,"context_line":"                if build.result is None:"},{"line_number":1737,"context_line":"                    build.result \u003d \u0027CANCELED\u0027"},{"line_number":1738,"context_line":"            else:"},{"line_number":1739,"context_line":"                nodeset \u003d buildset.getJobNodeSet(job_name)"},{"line_number":1740,"context_line":"                if nodeset:"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_3500c067","line":1737,"updated":"2020-05-28 21:55:20.000000000","message":"Why did this need to change?","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"86c31ce557648b3568c374340a1e779b33c1f447","unresolved":false,"context_lines":[{"line_number":1734,"context_line":"                        self.nodepool.returnNodeSet("},{"line_number":1735,"context_line":"                            nodeset, build\u003dbuild, zuul_event_id\u003ditem.event)"},{"line_number":1736,"context_line":"                if build.result is None:"},{"line_number":1737,"context_line":"                    build.result \u003d \u0027CANCELED\u0027"},{"line_number":1738,"context_line":"            else:"},{"line_number":1739,"context_line":"                nodeset \u003d buildset.getJobNodeSet(job_name)"},{"line_number":1740,"context_line":"                if nodeset:"}],"source_content_type":"text/x-python","patch_set":29,"id":"ff570b3c_351fee6c","line":1737,"in_reply_to":"ff570b3c_3500c067","updated":"2020-06-02 12:50:09.000000000","message":"For items in a failing bundle the cancelJobs() might also be called for the item that caused the bundle to fail.\n\nI also can\u0027t think of a reason to change this to CANCELED when there is already a result available and the job wasn\u0027t really canceled.","commit_id":"57ab1aafe7bd01fdde610323827661724513ebc3"}]}