)]}'
{"doc/source/review-the-openstack-way.rst":[{"author":{"_account_id":10607,"name":"Alexandra Settle","email":"a.settle@outlook.com","username":"asettle"},"change_message_id":"bdc049dc188967d356e70442bb214bc7936cc6b1","unresolved":false,"context_lines":[{"line_number":148,"context_line":"Procedural Code Review -2"},{"line_number":149,"context_line":"-------------------------"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Sometimes it\u0027s convenient to rely on the core-reviewer-only availability,"},{"line_number":152,"context_line":"persistence over future patches and merge-blocking behavior of a Code Review -2"},{"line_number":153,"context_line":"to temporarily prevent any other core reviewers from successfully approving a"},{"line_number":154,"context_line":"change even if the change itself is a desirable one."}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_a42e1bf8","line":151,"range":{"start_line":151,"start_character":72,"end_line":151,"end_character":73},"updated":"2019-06-17 15:06:40.000000000","message":"is this a misplaced comma? It doesn\u0027t appear to fit","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":10607,"name":"Alexandra Settle","email":"a.settle@outlook.com","username":"asettle"},"change_message_id":"bdc049dc188967d356e70442bb214bc7936cc6b1","unresolved":false,"context_lines":[{"line_number":148,"context_line":"Procedural Code Review -2"},{"line_number":149,"context_line":"-------------------------"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Sometimes it\u0027s convenient to rely on the core-reviewer-only availability,"},{"line_number":152,"context_line":"persistence over future patches and merge-blocking behavior of a Code Review -2"},{"line_number":153,"context_line":"to temporarily prevent any other core reviewers from successfully approving a"},{"line_number":154,"context_line":"change even if the change itself is a desirable one."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"For example, some projects will put a -2 vote on feature changes after Feature"},{"line_number":157,"context_line":"Freeze and before branching for the next release, to ensure that no features"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_e443b3ae","line":154,"range":{"start_line":151,"start_character":0,"end_line":154,"end_character":52},"updated":"2019-06-17 15:06:40.000000000","message":"\"Occasionally it is convenient to rely on the core-reviewer-only availability. Persistence over future patches and merge-blocking behavior of a Code Review -2 to temporarily prevent any other core reviewers from successfully approving a change even if the change itself is a desirable one...\" (and then it feels like there should be something here?","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"51c38f54fe8b5e09ed2acd8c48ac8d68e4a67e6d","unresolved":false,"context_lines":[{"line_number":148,"context_line":"Procedural Code Review -2"},{"line_number":149,"context_line":"-------------------------"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Sometimes it\u0027s convenient to rely on the core-reviewer-only availability,"},{"line_number":152,"context_line":"persistence over future patches and merge-blocking behavior of a Code Review -2"},{"line_number":153,"context_line":"to temporarily prevent any other core reviewers from successfully approving a"},{"line_number":154,"context_line":"change even if the change itself is a desirable one."},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"For example, some projects will put a -2 vote on feature changes after Feature"},{"line_number":157,"context_line":"Freeze and before branching for the next release, to ensure that no features"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_6e9b7416","line":154,"range":{"start_line":151,"start_character":0,"end_line":154,"end_character":52},"in_reply_to":"9fb8cfa7_e443b3ae","updated":"2019-06-27 09:42:20.000000000","message":"Still very hard to grok sentence.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"3112d4aac3b573481ad859b9e2e70c995293b4a2","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"For example, some projects will put a -2 vote on feature changes after Feature"},{"line_number":157,"context_line":"Freeze and before branching for the next release, to ensure that no features"},{"line_number":158,"context_line":"are unintentionally merged during the freeze. The person who added these -2s"},{"line_number":159,"context_line":"will then remove them again once the master branch is open for new features."},{"line_number":160,"context_line":"They should leave a comment explaining exactly what is happening. Submitters"},{"line_number":161,"context_line":"can continue to revise the change during the freeze."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_31609ea5","line":160,"range":{"start_line":158,"start_character":46,"end_line":160,"end_character":65},"updated":"2019-02-25 21:58:22.000000000","message":"I feel it\u0027s important to make clear that these requirements (always explain what\u0027s happening; always remove the vote once the temporary condition has passed) apply whatever the reason for the procedural -2. We might need to refactor the text a little to do that.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"d0be8703c6a09e395bb525bf0c0a9f323151effc","unresolved":false,"context_lines":[{"line_number":157,"context_line":"Freeze and before branching for the next release, to ensure that no features"},{"line_number":158,"context_line":"are unintentionally merged during the freeze. The person who added these -2s"},{"line_number":159,"context_line":"will then remove them again once the master branch is open for new features."},{"line_number":160,"context_line":"They should leave a comment explaining exactly what is happening. Submitters"},{"line_number":161,"context_line":"can continue to revise the change during the freeze."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_ba5f833f","line":160,"updated":"2019-02-26 00:22:12.000000000","message":"This is a great point, I\u0027m open to further adjustment of the existing sections to reflect this.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"51c38f54fe8b5e09ed2acd8c48ac8d68e4a67e6d","unresolved":false,"context_lines":[{"line_number":155,"context_line":""},{"line_number":156,"context_line":"For example, some projects will put a -2 vote on feature changes after Feature"},{"line_number":157,"context_line":"Freeze and before branching for the next release, to ensure that no features"},{"line_number":158,"context_line":"are unintentionally merged during the freeze. The person who added these -2s"},{"line_number":159,"context_line":"will then remove them again once the master branch is open for new features."},{"line_number":160,"context_line":"They should leave a comment explaining exactly what is happening. Submitters"},{"line_number":161,"context_line":"can continue to revise the change during the freeze."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_4e01d037","line":160,"range":{"start_line":158,"start_character":46,"end_line":160,"end_character":65},"in_reply_to":"9fdfeff1_31609ea5","updated":"2019-06-27 09:42:20.000000000","message":"agreed","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":10607,"name":"Alexandra Settle","email":"a.settle@outlook.com","username":"asettle"},"change_message_id":"bdc049dc188967d356e70442bb214bc7936cc6b1","unresolved":false,"context_lines":[{"line_number":160,"context_line":"They should leave a comment explaining exactly what is happening. Submitters"},{"line_number":161,"context_line":"can continue to revise the change during the freeze."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_8412f7b3","line":163,"range":{"start_line":163,"start_character":22,"end_line":163,"end_character":27},"updated":"2019-06-17 15:06:40.000000000","message":"mean","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"3112d4aac3b573481ad859b9e2e70c995293b4a2","unresolved":false,"context_lines":[{"line_number":160,"context_line":"They should leave a comment explaining exactly what is happening. Submitters"},{"line_number":161,"context_line":"can continue to revise the change during the freeze."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_91662abb","line":164,"range":{"start_line":163,"start_character":31,"end_line":164,"end_character":67},"updated":"2019-02-25 21:58:22.000000000","message":"This sounds like a special case of a feature freeze, so +1 to documenting it.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"6e54f0b1054c6b8a43465ec4b4c191bfeb990dd4","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."},{"line_number":168,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_0268411e","line":165,"updated":"2019-03-01 17:00:23.000000000","message":"Cool, this makes a really good case for mentioning the Priority label some teams have started to use, since a Priority -1 vote can also be used as a persistent merge blocker instead of reusing Code-Review -2 for that purpose.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"3112d4aac3b573481ad859b9e2e70c995293b4a2","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."},{"line_number":168,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_11b31aeb","line":165,"range":{"start_line":165,"start_character":0,"end_line":165,"end_character":75},"updated":"2019-02-25 21:58:22.000000000","message":"Endorsing this makes me nervous TBH. What\u0027s wrong with \"-1, I have some concerns, let\u0027s discuss this on the mailing list here: ...\"?\n\n-2ing a patch is just about the most unfriendly thing you can do to a new, or any, contributor (\"If I ever got a -2 on one of my patches... I would cry\" --actual thing once said to me by a new contributor), so there had better be a good reason for it.\n\nIf core reviewers can\u0027t trust each other to read previous comments before reviewing a patch then maybe that\u0027s a problem that should be solved between them rather than by hitting contributors with ever-larger hammers?","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"d0be8703c6a09e395bb525bf0c0a9f323151effc","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."},{"line_number":168,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_da5cc74c","line":165,"updated":"2019-02-26 00:22:12.000000000","message":"This is a fair point. While it does happen from time to time, it\u0027s perhaps not a case we\u0027d want to actively endorse.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":7104,"name":"Curtis Collicutt","email":"curtis@serverascode.com","username":"serverascode"},"change_message_id":"1c78254642a33a23a6eef931f8a55c7ab57e0b3e","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."},{"line_number":168,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_849a58c2","line":165,"range":{"start_line":165,"start_character":0,"end_line":165,"end_character":75},"in_reply_to":"9fdfeff1_11b31aeb","updated":"2019-02-26 15:29:24.000000000","message":"I agree with Zane. I believe the initiator of this patch was a discussion on the starlingx mailing list which is this *exact* case, a new contributor was given a -2. I personally don\u0027t think that is appropriate and would heavily suggest not codifying that.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":308,"name":"Thierry Carrez","email":"thierry@openstack.org","username":"ttx"},"change_message_id":"8f3927d75e192d4ff097ee3322a9d1005fa19f9f","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"This is also a common means of blocking changes associated with an approved"},{"line_number":164,"context_line":"specification which has been deferred to a future development cycle, or even"},{"line_number":165,"context_line":"just holding them until a necessary discussion has been concluded elsewhere. It"},{"line_number":166,"context_line":"is similar in some ways to setting `Workflow -1`_ except that it can\u0027t be"},{"line_number":167,"context_line":"accidentally removed by the change author when pushing a new patch."},{"line_number":168,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_2ea774be","line":165,"range":{"start_line":165,"start_character":0,"end_line":165,"end_character":75},"in_reply_to":"9fdfeff1_849a58c2","updated":"2019-03-01 14:28:09.000000000","message":"Agreed, there is a \"never in a million years\" subtone to -2 -- we should keep those for cases where merging would be damaging (or hard to rollback), with proper explanation. I prefer the occasional revert in case someone accidentally bypasses a -1, than the negative message a casual use of a -2 sends.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":4257,"name":"Zane Bitter","email":"zbitter@redhat.com","username":"zaneb"},"change_message_id":"3112d4aac3b573481ad859b9e2e70c995293b4a2","unresolved":false,"context_lines":[{"line_number":181,"context_line":"Core reviewers may also use the Workflow -1 vote to prevent a change from being"},{"line_number":182,"context_line":"merged during some temporary condition, without interrupting the code-review"},{"line_number":183,"context_line":"process. This does not persist through future patches, however, so if that is"},{"line_number":184,"context_line":"the intent a `Procedural Code Review -2`_ may be preferred."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":".. _follow-up-changes:"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_711dc6bc","line":184,"updated":"2019-02-25 21:58:22.000000000","message":"Maybe we need a workflow -2 (like workflow -1 but sticky)?\n\nEven if it is in effect the same thing as Code Review -2. Any vote labelled \"Code Review\" is likely to be interpreted by contributors as a judgement of their code. The more I think about it, the more it seems wrong to use it for workflow management purposes at all.\n\n(Another benefit is that a core reviewer would be able to code review a patch even if they were also e.g. the person in charge of holding features back during a freeze, without ambiguity.)","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"d0be8703c6a09e395bb525bf0c0a9f323151effc","unresolved":false,"context_lines":[{"line_number":181,"context_line":"Core reviewers may also use the Workflow -1 vote to prevent a change from being"},{"line_number":182,"context_line":"merged during some temporary condition, without interrupting the code-review"},{"line_number":183,"context_line":"process. This does not persist through future patches, however, so if that is"},{"line_number":184,"context_line":"the intent a `Procedural Code Review -2`_ may be preferred."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":".. _follow-up-changes:"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fdfeff1_7a559b1d","line":184,"updated":"2019-02-26 00:22:12.000000000","message":"The challenge there is that if a project adds -2 to the Workflow range, Gerrit will no longer consider -1 to prevent merging. See https://review.openstack.org/Documentation/config-labels.html for the label configuration options we have at our disposal right now (at least if we don\u0027t resort to writing prolog rules). Otherwise I agree this would be a nifty solution. A similar alternative would be to add a separate \"Procedural\" label with a -1..0 range limited to core reviewers.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"},{"author":{"_account_id":17068,"name":"Jean-Philippe Evrard","email":"openstack@a.spamming.party","username":"evrardjp"},"change_message_id":"51c38f54fe8b5e09ed2acd8c48ac8d68e4a67e6d","unresolved":false,"context_lines":[{"line_number":181,"context_line":"Core reviewers may also use the Workflow -1 vote to prevent a change from being"},{"line_number":182,"context_line":"merged during some temporary condition, without interrupting the code-review"},{"line_number":183,"context_line":"process. This does not persist through future patches, however, so if that is"},{"line_number":184,"context_line":"the intent a `Procedural Code Review -2`_ may be preferred."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":".. _follow-up-changes:"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"9fb8cfa7_4e0b1029","line":184,"in_reply_to":"9fdfeff1_7a559b1d","updated":"2019-06-27 09:42:20.000000000","message":"I am not sure Procedural even needs to stay only for cores.\n\nThere are patchsets you may want to prevent merging until some things happen, and which are neither Work in progress nor Depends-On another task.\nSo they deserve a Workflow +1 Code review+x and Procedural -1 that sticks through rebases.\n\nI would be in favour of adding a \"Procedural\" -1 that the owner of the patch and cores could adapt.","commit_id":"af0d1436f320ecd7fab2561ce2c3b75a380820bf"}]}