)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"7d854244773e2dfdd639bd7e96454c454ae95589","unresolved":true,"context_lines":[{"line_number":27,"context_line":"Yaml) as its value."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"I don\u0027t want to do that as part of this review since that would be a"},{"line_number":30,"context_line":"breaking change."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#review-input"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"31702c5f_687c07fc","line":30,"updated":"2025-01-07 17:02:33.000000000","message":"(Note I\u0027m not actually prescribing this I just want to brainstorm approaches and their drawbacks/benefits. I think this is similar to what fungi more briefly described but I wanted to be more explicit.)\n\nOne way to make this backward and forward compatible would be to use a different datastructure. Instead of:\n\n```\ngerrit:\n  notify: ALL\n  Verified: 1\n```\n\nYou could do something like:\n\n```\ngerrit:\n  Verified:\n    value: 1\n    notify: ALL\n```\n\nWhile still accepting the old format that uses default notification levels.\n\nThe problem with this approach is what do you do with:\n\n```\ngerrit:\n  Verified:\n    value: 1\n    notify: ALL\n  Workflow:\n    value: 1\n    notify: NONE\n```\n\nI believe we currently apply all labels associated with the result at once (which can be important to avoid race conditions). And in this example NONE and ALL are in conflict with one another.\n\nAnother options might be to try and make the string less likely to conflict. Something like `_notify: ALL` maybe?\n\nMaybe we should add a new top level dictionary that can carry all of the desired ReviewInput fields even if we only support notify to start (this would address the idea that all of those fields could be exposed eventually without needing to rearrange stuff over time and handle backward compatibility as we do). For example\n\n```\ngerrit:\n  _review_input:\n    notify: ALL\n  Verified: 1\n```","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"9906e30e08769cd3a11d8a1a5b323d376b99c6cd","unresolved":false,"context_lines":[{"line_number":27,"context_line":"Yaml) as its value."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"I don\u0027t want to do that as part of this review since that would be a"},{"line_number":30,"context_line":"breaking change."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#review-input"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ea7adc21_70994ffe","line":30,"in_reply_to":"31702c5f_687c07fc","updated":"2025-01-07 17:23:10.000000000","message":"Clark wrote this before I posted my message which was an attempt to avoid people spending time on something I don\u0027t think we need or want to do.  But since it\u0027s out there now; if we did want to change the structure, it would have to be something like:\n\n```\n  gerrit:\n    labels:\n      Verified: 1\n    notify: NONE\n```\n\nBecause of the point you raise in example #3.  We can handle the upgrade if we need to.\n\nBut, again, the current system is working as designed and there\u0027s no need to change it unless we get a lot more of these.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":37624,"name":"John Soo","display_name":"John Soo","email":"john.soo+ubuntu-one@arista.com","username":"john.soo"},"change_message_id":"c902b2bacc13bb6a5d322a8c4ba08b6672e3c533","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"2887e748_94f54c10","updated":"2025-01-06 19:46:42.000000000","message":"Thanks @fungi@yuggoth.org - do I need to do anything else to get this merged?\n\nOne other point I thought of re:replacing flat list of labels is that the ssh command does not accept all the review input fields that the http api does. I still would love to have access to all of them as a pipeline auther, if possible, though.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"7d854244773e2dfdd639bd7e96454c454ae95589","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b32b3973_0b0b02ad","in_reply_to":"2887e748_94f54c10","updated":"2025-01-07 17:02:33.000000000","message":"\u003e One other point I thought of re:replacing flat list of labels is that the ssh command does not accept all the review input fields that the http api does. I still would love to have access to all of them as a pipeline auther, if possible, though.\n\nIn general Zuul does still currently support the use of ssh without http; however, with reduced functionality. For example I believe that inline commenting is limited to configuring an http connection.\n\nI think if the intention is to expose all of the input fields then I\u0027m not sure the approach in this change is sufficient. It is simple which is nice for this specific field but once we start adding others I think this backs us into a corner of backward compatibility tech debt that might be best addressed once.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"}],"doc/source/drivers/gerrit.rst":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"be2921774b06d1a1c7df630be7385345a427606f","unresolved":false,"context_lines":[{"line_number":531,"context_line":"      if present)."},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"   .. attr:: notify"},{"line_number":534,"context_line":"      :default: None"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"      If this is set to a notify handling value then send"},{"line_number":537,"context_line":"      notifications at the specified level. If not, use the default"}],"source_content_type":"text/x-rst","patch_set":11,"id":"01f3b70b_b433cdd8","line":534,"updated":"2025-01-07 16:50:22.000000000","message":"This is just a tiny bit ambiguous, relying on the distinction between \"None\" and \"NONE\" to suggest that Zuul does not supply a default.  I think it would be best to omit the \":default:\" tag here; the text below indicates that we delegate the default handling to Gerrit (which I think is appropriate; I don\u0027t think we should say \"ALL\" here).\n\n(Usually we try to keep these values to literal yaml values; sometimes we put in explanatory texts.  If you really want something here, we could have it say \":default: The Gerrit default behavior\".  But I think it\u0027s more clear to omit.)","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":37624,"name":"John Soo","display_name":"John Soo","email":"john.soo+ubuntu-one@arista.com","username":"john.soo"},"change_message_id":"8417f9a2a17befe77d40db6bb504b0c016bb5e51","unresolved":false,"context_lines":[{"line_number":531,"context_line":"      if present)."},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"   .. attr:: notify"},{"line_number":534,"context_line":"      :default: None"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"      If this is set to a notify handling value then send"},{"line_number":537,"context_line":"      notifications at the specified level. If not, use the default"}],"source_content_type":"text/x-rst","patch_set":11,"id":"f0f04c85_31896cfe","line":534,"in_reply_to":"01f3b70b_b433cdd8","updated":"2025-01-07 19:15:59.000000000","message":"Ah, very good point. I\u0027ve omitted the `None` from patchset++.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"}],"doc/source/examples/pipelines/gerrit-reference-pipelines.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"7d854244773e2dfdd639bd7e96454c454ae95589","unresolved":true,"context_lines":[{"line_number":17,"context_line":"          comment: (?i)^(Patch Set [0-9]+:)?( [\\w\\\\+-]*)*(\\n\\n)?\\s*recheck"},{"line_number":18,"context_line":"    success:"},{"line_number":19,"context_line":"      gerrit:"},{"line_number":20,"context_line":"        notify: NONE"},{"line_number":21,"context_line":"        # Note that gerrit keywords are case-sensitive."},{"line_number":22,"context_line":"        Verified: 1"},{"line_number":23,"context_line":"    failure:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"1ed2d026_32173e13","line":20,"updated":"2025-01-07 17:02:33.000000000","message":"I believe that these examples are meant to be directly usable by others setting up Zuul pipelines. I\u0027m not sure that a non default notification setting here is appropriate as a result.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":37624,"name":"John Soo","display_name":"John Soo","email":"john.soo+ubuntu-one@arista.com","username":"john.soo"},"change_message_id":"b47b3ce43a52784e2417c6878c0257b6030861c5","unresolved":true,"context_lines":[{"line_number":17,"context_line":"          comment: (?i)^(Patch Set [0-9]+:)?( [\\w\\\\+-]*)*(\\n\\n)?\\s*recheck"},{"line_number":18,"context_line":"    success:"},{"line_number":19,"context_line":"      gerrit:"},{"line_number":20,"context_line":"        notify: NONE"},{"line_number":21,"context_line":"        # Note that gerrit keywords are case-sensitive."},{"line_number":22,"context_line":"        Verified: 1"},{"line_number":23,"context_line":"    failure:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"661f7617_2b3acbd2","line":20,"in_reply_to":"1ed2d026_32173e13","updated":"2025-01-07 19:20:37.000000000","message":"I have no strong feelings on the matter. But since this gets inlined into the documentation I thought it would be helpful to demonstrate what the `notify` field would look like.\n\nThere are a variety of further examples down below without it, too. What do you think?","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"8bf1e40ec33660d081ac5bdeb69327724885ac4f","unresolved":true,"context_lines":[{"line_number":17,"context_line":"          comment: (?i)^(Patch Set [0-9]+:)?( [\\w\\\\+-]*)*(\\n\\n)?\\s*recheck"},{"line_number":18,"context_line":"    success:"},{"line_number":19,"context_line":"      gerrit:"},{"line_number":20,"context_line":"        notify: NONE"},{"line_number":21,"context_line":"        # Note that gerrit keywords are case-sensitive."},{"line_number":22,"context_line":"        Verified: 1"},{"line_number":23,"context_line":"    failure:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"9d4788a0_1949f25a","line":20,"in_reply_to":"661f7617_2b3acbd2","updated":"2025-01-07 19:38:51.000000000","message":"I agree with Clark we shouldn\u0027t put this in the reference pipelines (it\u0027s \"reference\" as in \"reference implementation\" not \"complete syntax reference\".  We kind of want people to start with these exact pipelines if they don\u0027t know what they\u0027re doing).\n\nIf you really want an example, feel free to put a snippet inline in the main documentation for the option.\n\nBut this option is a simple text string, it\u0027s not expected most people would use it, and it has peers like \"submit\" with existing clear examples, so I think we can live without an example.","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"},{"author":{"_account_id":37624,"name":"John Soo","display_name":"John Soo","email":"john.soo+ubuntu-one@arista.com","username":"john.soo"},"change_message_id":"3837b626a13f777c442ec53e0acdcfcd285c497a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"          comment: (?i)^(Patch Set [0-9]+:)?( [\\w\\\\+-]*)*(\\n\\n)?\\s*recheck"},{"line_number":18,"context_line":"    success:"},{"line_number":19,"context_line":"      gerrit:"},{"line_number":20,"context_line":"        notify: NONE"},{"line_number":21,"context_line":"        # Note that gerrit keywords are case-sensitive."},{"line_number":22,"context_line":"        Verified: 1"},{"line_number":23,"context_line":"    failure:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"71d0070a_331b680b","line":20,"in_reply_to":"9d4788a0_1949f25a","updated":"2025-01-07 20:15:03.000000000","message":"Done. Thanks for the clarity and great review :)","commit_id":"e14e448828cfbf9efcd9fbffa13876e58be7ec24"}]}