)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"162b510f_f29a8ed6","updated":"2022-04-06 00:00:05.000000000","message":"Adding -1 for results of proofreading.\n\nOne other thing, this is very well buried. We have a lot of docs and which ones are relevant to new contributors is not self-evident. This essential reading is in \"Other Resources\". Not sure if anything can be done about this, but wanted to mention it.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"b39eda702fb2a697e6c3c87362b38c049a33056f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"781dde29_02c1c472","updated":"2022-04-29 14:42:33.000000000","message":"Couple of in-line comments.  They can be addressed in a follow-up change or if you push up another patch set.","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e726d97c2c1427820a67bf1dc2b2049f90491334","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"069c870b_2c22583d","updated":"2022-04-29 19:55:36.000000000","message":"Good additional info, Rajat!  I think let\u0027s merge this and then you can hit some of the points various reviewers have made in a follow-up.","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1b9424b7_013a5081","updated":"2022-04-22 14:31:38.000000000","message":"Thanks Eric and Pete for the review.","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"288b770fed30db4bd738bf2a50b7de3fa9858e93","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f047eb1c_3ae367c4","updated":"2022-04-29 14:21:33.000000000","message":"Thanks Rajat","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"}],"doc/source/contributor/gerrit.rst":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":191,"context_line":"  about how to fix it then we can provide valuable guidance on it."},{"line_number":192,"context_line":"  There are also jobs specific to particular area of code (for example,"},{"line_number":193,"context_line":"  ``cinder-plugin-ceph-tempest`` for the RBD volume driver,"},{"line_number":194,"context_line":"  ``devstack-plugin-nfs-tempest-full`` for the generic NFS driver etc) and look"},{"line_number":195,"context_line":"  for issues in the jobs if they are related to the code changes proposed."},{"line_number":196,"context_line":"  There is a past example on why we should check these jobs, the"},{"line_number":197,"context_line":"  ``devstack-plugin-nfs-tempest-full`` is a non-voting job and was failing on"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9af07d34_18a18290","line":194,"range":{"start_line":194,"start_character":71,"end_line":194,"end_character":74},"updated":"2022-04-06 00:00:05.000000000","message":"Did you mean \", so\"? As in \"There are also jobs (such and such), so look for issues.\"","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":191,"context_line":"  about how to fix it then we can provide valuable guidance on it."},{"line_number":192,"context_line":"  There are also jobs specific to particular area of code (for example,"},{"line_number":193,"context_line":"  ``cinder-plugin-ceph-tempest`` for the RBD volume driver,"},{"line_number":194,"context_line":"  ``devstack-plugin-nfs-tempest-full`` for the generic NFS driver etc) and look"},{"line_number":195,"context_line":"  for issues in the jobs if they are related to the code changes proposed."},{"line_number":196,"context_line":"  There is a past example on why we should check these jobs, the"},{"line_number":197,"context_line":"  ``devstack-plugin-nfs-tempest-full`` is a non-voting job and was failing on"}],"source_content_type":"text/x-rst","patch_set":3,"id":"cce8634f_42ce37b3","line":194,"range":{"start_line":194,"start_character":71,"end_line":194,"end_character":74},"in_reply_to":"9af07d34_18a18290","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":215,"context_line":"* Readability: In a large codebase (like Cinder), Readability is a big factor"},{"line_number":216,"context_line":"  as remembering the logic of every code path is not feasible and contributors"},{"line_number":217,"context_line":"  change from time to time. We should adapt to writing readable code which is"},{"line_number":218,"context_line":"  easy to follow and can be understood by anyone having knowledge about python"},{"line_number":219,"context_line":"  constructs and working of Cinder. Sometimes it happens that a logic can only"},{"line_number":220,"context_line":"  be written in a complex way, in that case, it\u0027s always good practice to add a"},{"line_number":221,"context_line":"  comment describing the functionality. So, if a logic proposed is not"}],"source_content_type":"text/x-rst","patch_set":3,"id":"a0fd032e_c2707c4d","line":218,"range":{"start_line":218,"start_character":72,"end_line":218,"end_character":78},"updated":"2022-04-06 00:00:05.000000000","message":"\"Python\"","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":215,"context_line":"* Readability: In a large codebase (like Cinder), Readability is a big factor"},{"line_number":216,"context_line":"  as remembering the logic of every code path is not feasible and contributors"},{"line_number":217,"context_line":"  change from time to time. We should adapt to writing readable code which is"},{"line_number":218,"context_line":"  easy to follow and can be understood by anyone having knowledge about python"},{"line_number":219,"context_line":"  constructs and working of Cinder. Sometimes it happens that a logic can only"},{"line_number":220,"context_line":"  be written in a complex way, in that case, it\u0027s always good practice to add a"},{"line_number":221,"context_line":"  comment describing the functionality. So, if a logic proposed is not"}],"source_content_type":"text/x-rst","patch_set":3,"id":"94e51a07_00c62011","line":218,"range":{"start_line":218,"start_character":72,"end_line":218,"end_character":78},"in_reply_to":"a0fd032e_c2707c4d","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"2ea30a0d0616eeddcfef0f78b6599adcfa263666","unresolved":true,"context_lines":[{"line_number":223,"context_line":"  feasible then asking for a comment that would explain it is also a valid"},{"line_number":224,"context_line":"  review point."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"}],"source_content_type":"text/x-rst","patch_set":3,"id":"d5ce0d51_838e4af5","line":226,"range":{"start_line":226,"start_character":2,"end_line":226,"end_character":6},"updated":"2022-04-05 17:03:00.000000000","message":"We should be clear that what we want is Python type annotations, and mypy happens to be the tool that we validate those with currently.  We could move to a different tool at some point, so we should probably just stay \"type annotations\" here.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":223,"context_line":"  feasible then asking for a comment that would explain it is also a valid"},{"line_number":224,"context_line":"  review point."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"}],"source_content_type":"text/x-rst","patch_set":3,"id":"4661885b_c3dc0285","line":226,"range":{"start_line":226,"start_character":2,"end_line":226,"end_character":6},"in_reply_to":"d5ce0d51_838e4af5","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":224,"context_line":"  review point."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"},{"line_number":230,"context_line":"  constructs."}],"source_content_type":"text/x-rst","patch_set":3,"id":"8a8888d0_23d7b642","line":227,"range":{"start_line":227,"start_character":2,"end_line":227,"end_character":8},"updated":"2022-04-06 00:00:05.000000000","message":"\"Cinder\", even if you mean the project name.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":224,"context_line":"  review point."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"},{"line_number":230,"context_line":"  constructs."}],"source_content_type":"text/x-rst","patch_set":3,"id":"8360ee54_1e7ca747","line":227,"range":{"start_line":227,"start_character":2,"end_line":227,"end_character":8},"in_reply_to":"8a8888d0_23d7b642","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"},{"line_number":230,"context_line":"  constructs."},{"line_number":231,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"dec900cf_b67b3f52","line":228,"range":{"start_line":228,"start_character":43,"end_line":228,"end_character":49},"updated":"2022-04-06 00:00:05.000000000","message":"\"Cinder\"","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"* mypy: There has been an ongoing effort to implement mypy standards all across"},{"line_number":227,"context_line":"  cinder. Certain areas of code already adapt to mypy coding style and it\u0027s"},{"line_number":228,"context_line":"  good practice that new code merging into cinder should also adapt to it."},{"line_number":229,"context_line":"  We, as reviewers, should ensure that new code proposed should include mypy"},{"line_number":230,"context_line":"  constructs."},{"line_number":231,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"b02e6bb4_8cb6a031","line":228,"range":{"start_line":228,"start_character":43,"end_line":228,"end_character":49},"in_reply_to":"dec900cf_b67b3f52","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"* Commit Message: There are few things that we should make sure the commit"},{"line_number":251,"context_line":"  message includes:"},{"line_number":252,"context_line":"  1) Make sure the author clearly explains in the commit message as to why the"},{"line_number":253,"context_line":"  code changes are necessary and how exactly the code changes fixing the issue"},{"line_number":254,"context_line":"  we have."},{"line_number":255,"context_line":"  2) It should have the appropriate tags (Eg: Closes-Bug, Related-Bug,"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3662894f_407f377e","line":252,"updated":"2022-04-06 00:00:05.000000000","message":"Each of these numbered points must be preceded by a blank line, else the document does not render correctly. Ironically, this issue makes this update contradict itself: I went to the output of Zuul to check the doc preview with this issue.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":249,"context_line":""},{"line_number":250,"context_line":"* Commit Message: There are few things that we should make sure the commit"},{"line_number":251,"context_line":"  message includes:"},{"line_number":252,"context_line":"  1) Make sure the author clearly explains in the commit message as to why the"},{"line_number":253,"context_line":"  code changes are necessary and how exactly the code changes fixing the issue"},{"line_number":254,"context_line":"  we have."},{"line_number":255,"context_line":"  2) It should have the appropriate tags (Eg: Closes-Bug, Related-Bug,"}],"source_content_type":"text/x-rst","patch_set":3,"id":"d4029cc7_f798a7c2","line":252,"in_reply_to":"3662894f_407f377e","updated":"2022-04-22 14:31:38.000000000","message":"It\u0027s still a WIP and I just jotted down the points to present the patch at the PTG, I will fix all the rendering issues in next PS but thanks for pointing this out, good to know that the doc is useful for it\u0027s own review :)","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":250,"context_line":"* Commit Message: There are few things that we should make sure the commit"},{"line_number":251,"context_line":"  message includes:"},{"line_number":252,"context_line":"  1) Make sure the author clearly explains in the commit message as to why the"},{"line_number":253,"context_line":"  code changes are necessary and how exactly the code changes fixing the issue"},{"line_number":254,"context_line":"  we have."},{"line_number":255,"context_line":"  2) It should have the appropriate tags (Eg: Closes-Bug, Related-Bug,"},{"line_number":256,"context_line":"  Blueprint, Depends-On etc). For detailed information refer to"}],"source_content_type":"text/x-rst","patch_set":3,"id":"b416468d_d559c487","line":253,"range":{"start_line":253,"start_character":62,"end_line":253,"end_character":68},"updated":"2022-04-06 00:00:05.000000000","message":"\"fix\"","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":250,"context_line":"* Commit Message: There are few things that we should make sure the commit"},{"line_number":251,"context_line":"  message includes:"},{"line_number":252,"context_line":"  1) Make sure the author clearly explains in the commit message as to why the"},{"line_number":253,"context_line":"  code changes are necessary and how exactly the code changes fixing the issue"},{"line_number":254,"context_line":"  we have."},{"line_number":255,"context_line":"  2) It should have the appropriate tags (Eg: Closes-Bug, Related-Bug,"},{"line_number":256,"context_line":"  Blueprint, Depends-On etc). For detailed information refer to"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c9df86bc_34998889","line":253,"range":{"start_line":253,"start_character":62,"end_line":253,"end_character":68},"in_reply_to":"b416468d_d559c487","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":270,"context_line":"* Ways of reviewing: There are various ways you can go about reviewing a patch,"},{"line_number":271,"context_line":"  following are some of the standard ways you can follow to provide valuable"},{"line_number":272,"context_line":"  feedback on the patch:"},{"line_number":273,"context_line":"  1) Testing it in local environment: The easiest way to check the correctness"},{"line_number":274,"context_line":"  of a code change proposed is to reproduce the issue (steps should be in"},{"line_number":275,"context_line":"  launchpad bug) and try the same steps after applying the patch to your"},{"line_number":276,"context_line":"  environment and see if the provided code changes fix the issue."}],"source_content_type":"text/x-rst","patch_set":3,"id":"dda144c3_616e0c7e","line":273,"updated":"2022-04-06 00:00:05.000000000","message":"Insert a blank line here too.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":270,"context_line":"* Ways of reviewing: There are various ways you can go about reviewing a patch,"},{"line_number":271,"context_line":"  following are some of the standard ways you can follow to provide valuable"},{"line_number":272,"context_line":"  feedback on the patch:"},{"line_number":273,"context_line":"  1) Testing it in local environment: The easiest way to check the correctness"},{"line_number":274,"context_line":"  of a code change proposed is to reproduce the issue (steps should be in"},{"line_number":275,"context_line":"  launchpad bug) and try the same steps after applying the patch to your"},{"line_number":276,"context_line":"  environment and see if the provided code changes fix the issue."}],"source_content_type":"text/x-rst","patch_set":3,"id":"607582b5_8321a521","line":273,"in_reply_to":"dda144c3_616e0c7e","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"2ea30a0d0616eeddcfef0f78b6599adcfa263666","unresolved":true,"context_lines":[{"line_number":280,"context_line":""},{"line_number":281,"context_line":"  2) Optimization: If you\u0027re not aware about the code path the patch is fixing,"},{"line_number":282,"context_line":"  you can still go ahead and provide valuable feedback about the python code"},{"line_number":283,"context_line":"  if that can be optimized to improve performance or reduce lines of code."},{"line_number":284,"context_line":"  Example: Following are two code snippets that perform the same task but one"},{"line_number":285,"context_line":"  is more optimized."},{"line_number":286,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"be10aa78_e6ccdd5b","line":283,"range":{"start_line":283,"start_character":50,"end_line":283,"end_character":73},"updated":"2022-04-05 17:03:00.000000000","message":"IMO it might not be wise to recommend code length as a priority for reviews.\n\nSometimes it\u0027s a win, but sometimes denser code can be much harder to read and harder to spot bugs in.  Maintainability of code is almost always higher priority for us than small performance differences.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":280,"context_line":""},{"line_number":281,"context_line":"  2) Optimization: If you\u0027re not aware about the code path the patch is fixing,"},{"line_number":282,"context_line":"  you can still go ahead and provide valuable feedback about the python code"},{"line_number":283,"context_line":"  if that can be optimized to improve performance or reduce lines of code."},{"line_number":284,"context_line":"  Example: Following are two code snippets that perform the same task but one"},{"line_number":285,"context_line":"  is more optimized."},{"line_number":286,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"290f3edf_91c0589e","line":283,"range":{"start_line":283,"start_character":50,"end_line":283,"end_character":73},"in_reply_to":"4f598cb2_9cec3313","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d646163bab01ccbced54ec8c56c67c6f0c08c43c","unresolved":true,"context_lines":[{"line_number":280,"context_line":""},{"line_number":281,"context_line":"  2) Optimization: If you\u0027re not aware about the code path the patch is fixing,"},{"line_number":282,"context_line":"  you can still go ahead and provide valuable feedback about the python code"},{"line_number":283,"context_line":"  if that can be optimized to improve performance or reduce lines of code."},{"line_number":284,"context_line":"  Example: Following are two code snippets that perform the same task but one"},{"line_number":285,"context_line":"  is more optimized."},{"line_number":286,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"4f598cb2_9cec3313","line":283,"range":{"start_line":283,"start_character":50,"end_line":283,"end_character":73},"in_reply_to":"be10aa78_e6ccdd5b","updated":"2022-04-06 00:00:05.000000000","message":"Agreed.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"2ea30a0d0616eeddcfef0f78b6599adcfa263666","unresolved":true,"context_lines":[{"line_number":311,"context_line":".. _Getting Started: https://docs.openstack.org/infra/manual/developers.html#getting-started"},{"line_number":312,"context_line":".. _Development Workflow: https://docs.openstack.org/infra/manual/developers.html#development-workflow"},{"line_number":313,"context_line":".. _patch: https://review.opendev.org/c/openstack/cinder/+/761152"},{"line_number":314,"context_line":".. _Microversion Doc: https://github.com/openstack/cinder/blob/master/doc/source/contributor/api_microversion_dev.rst#other-necessary-changes"},{"line_number":315,"context_line":".. _external references in commit message: https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references"},{"line_number":316,"context_line":".. _Summary of Git commit message structure: https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure"},{"line_number":317,"context_line":".. _Release notes: https://docs.openstack.org/cinder/latest/contributor/releasenotes.html"}],"source_content_type":"text/x-rst","patch_set":3,"id":"23ff22b0_6992972e","line":314,"range":{"start_line":314,"start_character":22,"end_line":314,"end_character":57},"updated":"2022-04-05 17:03:00.000000000","message":"Link to opendev instead of github in our docs.","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c488182565051ec897c23b69822ff6a2825f7e20","unresolved":false,"context_lines":[{"line_number":311,"context_line":".. _Getting Started: https://docs.openstack.org/infra/manual/developers.html#getting-started"},{"line_number":312,"context_line":".. _Development Workflow: https://docs.openstack.org/infra/manual/developers.html#development-workflow"},{"line_number":313,"context_line":".. _patch: https://review.opendev.org/c/openstack/cinder/+/761152"},{"line_number":314,"context_line":".. _Microversion Doc: https://github.com/openstack/cinder/blob/master/doc/source/contributor/api_microversion_dev.rst#other-necessary-changes"},{"line_number":315,"context_line":".. _external references in commit message: https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references"},{"line_number":316,"context_line":".. _Summary of Git commit message structure: https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure"},{"line_number":317,"context_line":".. _Release notes: https://docs.openstack.org/cinder/latest/contributor/releasenotes.html"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9053aef9_56b7fd80","line":314,"range":{"start_line":314,"start_character":22,"end_line":314,"end_character":57},"in_reply_to":"23ff22b0_6992972e","updated":"2022-04-22 14:31:38.000000000","message":"Done","commit_id":"7d9b65d7ed3e345d8de038ba7b19e4add7079af3"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e726d97c2c1427820a67bf1dc2b2049f90491334","unresolved":true,"context_lines":[{"line_number":59,"context_line":"  that backports may be more problematic, and hence making sure that we have"},{"line_number":60,"context_line":"  really good test coverage."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"Targeting Milestones"},{"line_number":63,"context_line":"--------------------"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"In an effort to guide team review priorities the Cinder team has"},{"line_number":66,"context_line":"adopted the process of adding comments to reviews to target a"},{"line_number":67,"context_line":"milestone for a particular patch.  This process is not required"},{"line_number":68,"context_line":"for all patches but is beneficial for patches that may be time sensitive."},{"line_number":69,"context_line":"For example patches that need to land earlier in the release cycle so as to"},{"line_number":70,"context_line":"get additional test time or because later development activities are dependent"},{"line_number":71,"context_line":"upon that functionality merging."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"To target a patch to a milestone a reviewer should add a comment using the"},{"line_number":74,"context_line":"following format:"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"  ``target-\u003crelease\u003e-\u003cmilestone\u003e``"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"Release should be used to indicate the release to which the patch should be"},{"line_number":79,"context_line":"targeted, all lower case.  The milestone is a single number, 1 to 3,"},{"line_number":80,"context_line":"indicating the milestone number. So, to target a patch to land in"},{"line_number":81,"context_line":"Milestone 2 of the Rocky release a comment like the following"},{"line_number":82,"context_line":"would be added:"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"  ``target-rocky-2``"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"Adding this tag allows reviewers to search for these tags and use them as a"},{"line_number":87,"context_line":"guide in review priorities."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Targeting patches should be done by Cinder Core Review Team members."},{"line_number":90,"context_line":"If a patch developer feels that a patch should be targeted to a"},{"line_number":91,"context_line":"milestone the developer should bring the request up to the Cinder"},{"line_number":92,"context_line":"team in a weekly meeting or on the ``#openstack-cinder`` IRC"},{"line_number":93,"context_line":"channel."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"Reviewing Vendor Patches"},{"line_number":96,"context_line":"------------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"8ea54f0b_7cdbf402","line":93,"range":{"start_line":62,"start_character":0,"end_line":93,"end_character":8},"updated":"2022-04-29 19:55:36.000000000","message":"We haven\u0027t used this targeting method in a long time; delete?","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"b39eda702fb2a697e6c3c87362b38c049a33056f","unresolved":true,"context_lines":[{"line_number":217,"context_line":"  factor as remembering the logic of every code path is not feasible and"},{"line_number":218,"context_line":"  contributors change from time to time. We should adapt to writing readable"},{"line_number":219,"context_line":"  code which is easy to follow and can be understood by anyone having"},{"line_number":220,"context_line":"  knowledge about Python constructs and working of Cinder. Sometimes it"},{"line_number":221,"context_line":"  happens that a logic can only be written in a complex way, in that case,"},{"line_number":222,"context_line":"  it\u0027s always good practice to add a comment describing the functionality."},{"line_number":223,"context_line":"  So, if a logic proposed is not readable, do ask/suggest a more readable"}],"source_content_type":"text/x-rst","patch_set":4,"id":"59c2d30a_542b6be3","line":220,"updated":"2022-04-29 14:42:33.000000000","message":"This line is awkward.  I think I would say \"... knowledge about Python constructs and the workings of Cinder.\"","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"b39eda702fb2a697e6c3c87362b38c049a33056f","unresolved":true,"context_lines":[{"line_number":267,"context_line":"  update the commit message leaving the commit describing the old changes."},{"line_number":268,"context_line":"  Verify that the commit message is updated as per code changes."},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"* **Release Notes**: There are different cases where a releasenote is required"},{"line_number":271,"context_line":"  like fixing a bug, adding a feature, changing areas affecting upgrade etc."},{"line_number":272,"context_line":"  You can refer to the `Release notes`_ section in our contributor docs for"},{"line_number":273,"context_line":"  more information."}],"source_content_type":"text/x-rst","patch_set":4,"id":"1968b207_5fc1dd2b","line":270,"updated":"2022-04-29 14:42:33.000000000","message":"We may want to add a note here that any vendor driver patches that plan to be backported should have a release note.  So, more or less, most vendor patches.","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"288b770fed30db4bd738bf2a50b7de3fa9858e93","unresolved":false,"context_lines":[{"line_number":288,"context_line":"  you can still go ahead and provide valuable feedback about the python code"},{"line_number":289,"context_line":"  if that can be optimized to improve maintainability or performance."},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"  3) Perform Dry Run: Sometimes the code changes are on code paths that we"},{"line_number":292,"context_line":"  don\u0027t have or can\u0027t create environment for (like vendor driver changes or"},{"line_number":293,"context_line":"  optional service changes like cinder-backup) so we can read through the code"},{"line_number":294,"context_line":"  or use some example values to perform a dry run of the code and see if it"}],"source_content_type":"text/x-rst","patch_set":4,"id":"ea75d552_307d71eb","line":291,"range":{"start_line":291,"start_character":2,"end_line":291,"end_character":21},"updated":"2022-04-29 14:21:33.000000000","message":"Cool","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e726d97c2c1427820a67bf1dc2b2049f90491334","unresolved":true,"context_lines":[{"line_number":297,"context_line":".. _Review guidelines: https://docs.openstack.org/doc-contrib-guide/docs-review-guidelines.html"},{"line_number":298,"context_line":".. _Gerrit: https://review.opendev.org/#/q/project:openstack/cinder+status:open"},{"line_number":299,"context_line":".. _Quick Reference: https://docs.openstack.org/infra/manual/developers.html#quick-reference"},{"line_number":300,"context_line":".. _Getting Started: https://docs.openstack.org/infra/manual/developers.html#getting-started"},{"line_number":301,"context_line":".. _Development Workflow: https://docs.openstack.org/infra/manual/developers.html#development-workflow"},{"line_number":302,"context_line":".. _patch: https://review.opendev.org/c/openstack/cinder/+/761152"},{"line_number":303,"context_line":".. _Microversion Doc: https://opendev.org/openstack/cinder/src/branch/master/doc/source/contributor/api_microversion_dev.rst#other-necessary-changes"}],"source_content_type":"text/x-rst","patch_set":4,"id":"68e9a7d5_17050b79","line":300,"range":{"start_line":300,"start_character":21,"end_line":300,"end_character":92},"updated":"2022-04-29 19:55:36.000000000","message":"For follow-up: the content was moved, this link should now be:\n\nhttps://docs.opendev.org/opendev/infra-manual/latest/gettingstarted.html","commit_id":"cf6e53b2ee3cd7dea29ad780b7a7e7358045f852"}]}