)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cd895c1e_931f1747","updated":"2022-02-14 09:32:21.000000000","message":"@Clay thanks for updating this doc - it definitely needs some improvements","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"345f3021_54d6b167","updated":"2022-02-12 14:49:14.000000000","message":"some of this info looked dated","commit_id":"baf5567cb053108319248428834666edbc33cc53"}],"REVIEW_GUIDELINES.rst":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"\"Did you even run this?\" is the review comment all contributors dread."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Reviewers in particular need to be fearful merging changes that just"},{"line_number":41,"context_line":"don\u0027t work - or at least fail in frequently common enough scenarios to"},{"line_number":42,"context_line":"be considered \"horribly broken\". A comment in our review that says"},{"line_number":43,"context_line":"roughly \"I ran this on my machine and observed ``description of"}],"source_content_type":"text/x-rst","patch_set":1,"id":"b729649d_312dded6","side":"PARENT","line":40,"updated":"2022-02-12 14:49:14.000000000","message":"don\u0027t be scared","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":42,"context_line":"be considered \"horribly broken\". A comment in our review that says"},{"line_number":43,"context_line":"roughly \"I ran this on my machine and observed ``description of"},{"line_number":44,"context_line":"behavior change is supposed to achieve``\" is the most powerful defense"},{"line_number":45,"context_line":"we have against the terrible scorn from our fellow Swift developers"},{"line_number":46,"context_line":"and operators when we accidentally merge bad code."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"If you\u0027re doing a fair amount of reviews - you will participate in"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a9911364_39e0351c","side":"PARENT","line":45,"updated":"2022-02-12 14:49:14.000000000","message":"there\u0027s no scorn","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":49,"context_line":"merging a change that will break my clusters - it\u0027s cool - I\u0027ll do it"},{"line_number":50,"context_line":"to you at some point too (sorry about that). But when either of us go"},{"line_number":51,"context_line":"look at the reviews to understand the process gap that allowed this to"},{"line_number":52,"context_line":"happen - it better not be just because we were too lazy to check it out"},{"line_number":53,"context_line":"and run it before it got merged."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Or be warned, you may receive, the dreaded..."}],"source_content_type":"text/x-rst","patch_set":1,"id":"a3a28452_c5415d0c","side":"PARENT","line":52,"updated":"2022-02-12 14:49:14.000000000","message":"i think we\u0027ve gotten pretty good at this - and if there\u0027s a gap it\u0027s not laziness","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    \"Did you even *run* this?\""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"I\u0027m sorry, I know it\u0027s rough. ;)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"Consider edge cases very seriously"},{"line_number":62,"context_line":"----------------------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"d01a745f_2c009d0c","side":"PARENT","line":59,"updated":"2022-02-12 14:49:14.000000000","message":"it\u0027s not so bad","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    -- Douglas Crockford"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Scale is an *amazingly* abusive partner. If you contribute changes to"},{"line_number":70,"context_line":"Swift your code is running - in production - at scale - and your bugs"},{"line_number":71,"context_line":"cannot hide. I wish on all of us that our bugs may be exceptionally"},{"line_number":72,"context_line":"rare - meaning they only happen in extremely unlikely edge cases. For"}],"source_content_type":"text/x-rst","patch_set":1,"id":"21899ae0_77caf164","side":"PARENT","line":69,"updated":"2022-02-12 14:49:14.000000000","message":"abusive sounds like a trigger word to me - it\u0027s just cat photos and self driving car footage - chill out.","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    -- Douglas Crockford"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Scale is an *amazingly* abusive partner. If you contribute changes to"},{"line_number":70,"context_line":"Swift your code is running - in production - at scale - and your bugs"},{"line_number":71,"context_line":"cannot hide. I wish on all of us that our bugs may be exceptionally"},{"line_number":72,"context_line":"rare - meaning they only happen in extremely unlikely edge cases. For"}],"source_content_type":"text/x-rst","patch_set":1,"id":"506ed82b_dd8ccb86","side":"PARENT","line":69,"in_reply_to":"21899ae0_77caf164","updated":"2022-02-14 09:32:21.000000000","message":"+1 agree, let\u0027s definitely remove this metaphor","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":222,"context_line":"adds a new feature, changes behavior, updates configs, or in any other"},{"line_number":223,"context_line":"way is different than previous behavior requires docs. manpages,"},{"line_number":224,"context_line":"sample configs, docstrings, descriptive prose in the source tree, etc."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Reviewers Write Code"},{"line_number":227,"context_line":"--------------------"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"51521854_42d53c50","side":"PARENT","line":225,"updated":"2022-02-12 14:49:14.000000000","message":"I dropped this because it said manpages, but maybe there\u0027s still more to say about docs...","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":222,"context_line":"adds a new feature, changes behavior, updates configs, or in any other"},{"line_number":223,"context_line":"way is different than previous behavior requires docs. manpages,"},{"line_number":224,"context_line":"sample configs, docstrings, descriptive prose in the source tree, etc."},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"Reviewers Write Code"},{"line_number":227,"context_line":"--------------------"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"d02c35b5_f4db5040","side":"PARENT","line":225,"in_reply_to":"51521854_42d53c50","updated":"2022-02-14 09:32:21.000000000","message":"We could add a checklist for docs e.g.:\n\n  Changes that add, remove or modify a config option must update the config file(s) and the deployment guide.\n\n  Changes that modify API behavior should update the API docs.\n\n  Changes that introduce a new feature should usually add an overview doc for the feature.\n\n  Changes that introduce or modify a development pattern may need to update development guidelines.","commit_id":"975d3dbcfe8ef7e8007444e5bb5c0b3f890e534e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":35,"context_line":"it locally and tries to test it, a severe and obvious error shows up."},{"line_number":36,"context_line":"Something like a syntax error or a missing dependency."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"\"Did you even run this?\" is a review comment that suggests the"},{"line_number":39,"context_line":"reviewer feels the author hasn\u0027t invested a fair amount of time"},{"line_number":40,"context_line":"exercising their change in a realistic way."},{"line_number":41,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"7c037593_d7b4231e","line":38,"range":{"start_line":38,"start_character":1,"end_line":38,"end_character":23},"updated":"2022-02-14 09:32:21.000000000","message":"we ought to be discouraging this kind of language in reviews, regardless of the quality of the patch - it\u0027s likely to be interpreted as snarky e.g. the use of \"even\" implies there are other shortcomings. \"Did you run this?\" would be less loaded, but \"I ran this and it failed in the following way\" is far less aggressive/accusatory.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":36,"context_line":"Something like a syntax error or a missing dependency."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"\"Did you even run this?\" is a review comment that suggests the"},{"line_number":39,"context_line":"reviewer feels the author hasn\u0027t invested a fair amount of time"},{"line_number":40,"context_line":"exercising their change in a realistic way."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"It\u0027s hard to produce code that works well at scale, a thourough review"},{"line_number":43,"context_line":"of a change requires that it has been *well* exercised - at *least* on"}],"source_content_type":"text/x-rst","patch_set":1,"id":"3a2f71e7_f4bd1e5c","line":40,"range":{"start_line":39,"start_character":9,"end_line":40,"end_character":42},"updated":"2022-02-14 09:32:21.000000000","message":"We need to be careful when straying into reviewing the author rather than reviewing the code: e.g. \"Your patch needs to be improved in the following ways...\" vs \"You didn\u0027t make enough effort\".","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":39,"context_line":"reviewer feels the author hasn\u0027t invested a fair amount of time"},{"line_number":40,"context_line":"exercising their change in a realistic way."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"It\u0027s hard to produce code that works well at scale, a thourough review"},{"line_number":43,"context_line":"of a change requires that it has been *well* exercised - at *least* on"},{"line_number":44,"context_line":"your dev machine; but maybe in prod. ;)"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"4b4da141_e0a8b6b4","line":42,"range":{"start_line":42,"start_character":50,"end_line":42,"end_character":51},"updated":"2022-02-13 06:56:08.000000000","message":"Full stop. Then, \"A thorough...\" (only one \"u\").","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"It\u0027s hard to produce code that works well at scale, a thourough review"},{"line_number":43,"context_line":"of a change requires that it has been *well* exercised - at *least* on"},{"line_number":44,"context_line":"your dev machine; but maybe in prod. ;)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"A comment on a review that says roughly \"I ran this in ``description"},{"line_number":47,"context_line":"of environment`` and observed ``description of behavior change is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"4d069cc2_686627bd","line":44,"range":{"start_line":44,"start_character":18,"end_line":44,"end_character":36},"updated":"2022-02-14 09:32:21.000000000","message":"It may not be realistic for every reviewer to test other people\u0027s patches in a production environment. I wouldn\u0027t want this comment to put someone off leaving a review.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"It\u0027s hard to produce code that works well at scale, a thourough review"},{"line_number":43,"context_line":"of a change requires that it has been *well* exercised - at *least* on"},{"line_number":44,"context_line":"your dev machine; but maybe in prod. ;)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"A comment on a review that says roughly \"I ran this in ``description"},{"line_number":47,"context_line":"of environment`` and observed ``description of behavior change is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7a0e7f99_5e747bbc","line":44,"range":{"start_line":44,"start_character":5,"end_line":44,"end_character":16},"updated":"2022-02-14 09:32:21.000000000","message":"a reference to SAIO here would be good","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"It\u0027s hard to produce code that works well at scale, a thourough review"},{"line_number":43,"context_line":"of a change requires that it has been *well* exercised - at *least* on"},{"line_number":44,"context_line":"your dev machine; but maybe in prod. ;)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"A comment on a review that says roughly \"I ran this in ``description"},{"line_number":47,"context_line":"of environment`` and observed ``description of behavior change is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"448610cb_171122c1","line":44,"range":{"start_line":44,"start_character":31,"end_line":44,"end_character":35},"updated":"2022-02-14 09:32:21.000000000","message":"s/prod/production/","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"A comment on a review that says roughly \"I ran this in ``description"},{"line_number":47,"context_line":"of environment`` and observed ``description of behavior change is"},{"line_number":48,"context_line":"supposed to achieve``\" is VERY useful."},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"Consider edge cases very seriously"},{"line_number":51,"context_line":"----------------------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"40472530_3cc19a49","line":48,"updated":"2022-02-13 06:56:08.000000000","message":"I feel like it might also be good to praise the review that\u0027s like\n\n\u003e I tested this in ``environment`` and it does what it says, but it breaks ``some relatively obscure workflow`` because of X which Ys then Z.\n\nBonus points when you\u0027re prepared to explain why the obscure workflow is in fact critical to your use-case :-)","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Scale is an *amazingly* helpful partner in producing robust systems."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"When contributing changes to Swift, our code is running - in"},{"line_number":61,"context_line":"production - at scale - and our bugs cannot hide."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Bad things that happen only 1 out of every 10K times an op is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"17a986ae_d777ffd7","line":60,"range":{"start_line":60,"start_character":36,"end_line":60,"end_character":39},"updated":"2022-02-13 06:56:08.000000000","message":"I think I preferred \"your\" -- yes, it becomes \"our\" shared code, but I felt like the point was to give the contributor some distinct sense of ownership. Otherwise, the \"when contributing changes to Swift\" lead-in feels a little odd.\n\nI\u0027d keep \"our bugs\", though -- once it\u0027s merged, the code is on all of us. Any bugs that came with it are not yours alone, though we\u0027ll likely want your input to make sure we\u0027re not horribly breaking the feature to fix the bug. These are two of the big up-shots to contributing upstream:\n\n* Your code has a high likelihood of being exercised at a scale you\u0027re not at yet -- so you\u0027ll have a higher degree of confidence that it will continue to work as you grow.\n* When there\u0027s a bug, it\u0027s all of ours to fix. You may still need to make the case for it, and you probably ought to be involved in the fix, but you\u0027ll have a great group of smart, helpful people backing you up.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Scale is an *amazingly* helpful partner in producing robust systems."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"When contributing changes to Swift, our code is running - in"},{"line_number":61,"context_line":"production - at scale - and our bugs cannot hide."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Bad things that happen only 1 out of every 10K times an op is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"feafad87_ec2f0513","line":60,"range":{"start_line":60,"start_character":36,"end_line":60,"end_character":39},"in_reply_to":"17a986ae_d777ffd7","updated":"2022-02-14 09:32:21.000000000","message":"or, avoid any implication of ownership and refer to \"the code\"\n\n  When contributing changes to Swift, remember that the code is running - in production - at scale - and bugs cannot hide.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":60,"context_line":"When contributing changes to Swift, our code is running - in"},{"line_number":61,"context_line":"production - at scale - and our bugs cannot hide."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Bad things that happen only 1 out of every 10K times an op is"},{"line_number":64,"context_line":"performed will be discovered in minutes. Bad things that happen only"},{"line_number":65,"context_line":"1 out of every one billion times something happens will be observed -"},{"line_number":66,"context_line":"by multiple deployments - over the course of a release. Bad things"}],"source_content_type":"text/x-rst","patch_set":1,"id":"f5c2121b_eb960dd3","line":63,"range":{"start_line":63,"start_character":56,"end_line":63,"end_character":58},"updated":"2022-02-13 06:56:08.000000000","message":"I\u0027d expand it to \"operation\"","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":72,"context_line":"Run the tests"},{"line_number":73,"context_line":"-------------"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"Yes, I know Zuul does this already. You can do it *too*. You might"},{"line_number":76,"context_line":"not need to re-run *all* the tests on your machine - it depends on the"},{"line_number":77,"context_line":"change. But, if you\u0027re not sure which will be most useful - running"},{"line_number":78,"context_line":"all of them could help use learn how to use them: unit, functional,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"27612cbc_48671be1","line":75,"updated":"2022-02-12 14:49:14.000000000","message":"keep up with the times!","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":76,"context_line":"not need to re-run *all* the tests on your machine - it depends on the"},{"line_number":77,"context_line":"change. But, if you\u0027re not sure which will be most useful - running"},{"line_number":78,"context_line":"all of them could help use learn how to use them: unit, functional,"},{"line_number":79,"context_line":"s3api, cors, probe. If you can\u0027t reliably"},{"line_number":80,"context_line":"get all tests passing in your development environment you will not be"},{"line_number":81,"context_line":"able to do as effective reviews. Whatever tests/suites you are able to"},{"line_number":82,"context_line":"exercise/validate on your machine against your config you should"}],"source_content_type":"text/x-rst","patch_set":1,"id":"138e1ebf_8881eaba","line":79,"updated":"2022-02-12 14:49:14.000000000","message":"love those cors tests","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":78,"context_line":"all of them could help use learn how to use them: unit, functional,"},{"line_number":79,"context_line":"s3api, cors, probe. If you can\u0027t reliably"},{"line_number":80,"context_line":"get all tests passing in your development environment you will not be"},{"line_number":81,"context_line":"able to do as effective reviews. Whatever tests/suites you are able to"},{"line_number":82,"context_line":"exercise/validate on your machine against your config you should"},{"line_number":83,"context_line":"mention in your review comments so that other reviewers might choose"},{"line_number":84,"context_line":"to do *other* testing locally when they have the change checked out."}],"source_content_type":"text/x-rst","patch_set":1,"id":"88678d1f_d1a35191","line":81,"range":{"start_line":81,"start_character":10,"end_line":81,"end_character":13},"updated":"2022-02-14 09:32:21.000000000","message":"I think it read better without the \u0027as\u0027","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3d1949912513a1ec87dd63a2d571647add2482a2","unresolved":true,"context_lines":[{"line_number":191,"context_line":"Documentation"},{"line_number":192,"context_line":"-------------"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"Most changes will include documentation. New functions and code"},{"line_number":195,"context_line":"should have Docstrings. Tests should obviate new or changed behaviors"},{"line_number":196,"context_line":"with descriptive and meaningful phrases. New features should include"},{"line_number":197,"context_line":"changes to the documentation tree. New config options should be"}],"source_content_type":"text/x-rst","patch_set":1,"id":"27225369_f7f0fbd2","line":194,"updated":"2022-02-12 14:49:14.000000000","message":"I think we can make this even stronger since we include docstrings, tests and ... commit messages when thinking about \"how is this change documented?\"","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":192,"context_line":"-------------"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"Most changes will include documentation. New functions and code"},{"line_number":195,"context_line":"should have Docstrings. Tests should obviate new or changed behaviors"},{"line_number":196,"context_line":"with descriptive and meaningful phrases. New features should include"},{"line_number":197,"context_line":"changes to the documentation tree. New config options should be"},{"line_number":198,"context_line":"documented in example configs. The commit message should document the"}],"source_content_type":"text/x-rst","patch_set":1,"id":"eaa35a24_a2f8d207","line":195,"range":{"start_line":195,"start_character":37,"end_line":195,"end_character":44},"updated":"2022-02-13 06:56:08.000000000","message":"While we\u0027re here -- I\u0027m not sure this is the word we want. \"Elucidate\", maybe? Or just \"demonstrate\". And maybe change \"with\" to \"using\".","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"707fdc8fdb5538469bdbc79f78845f6cb739ef64","unresolved":true,"context_lines":[{"line_number":194,"context_line":"Most changes will include documentation. New functions and code"},{"line_number":195,"context_line":"should have Docstrings. Tests should obviate new or changed behaviors"},{"line_number":196,"context_line":"with descriptive and meaningful phrases. New features should include"},{"line_number":197,"context_line":"changes to the documentation tree. New config options should be"},{"line_number":198,"context_line":"documented in example configs. The commit message should document the"},{"line_number":199,"context_line":"change for the change log."},{"line_number":200,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"a4f1bffb_83f46cb4","line":197,"range":{"start_line":197,"start_character":15,"end_line":197,"end_character":33},"updated":"2022-02-13 06:56:08.000000000","message":"Might be worth pointing out that module-level docstrings are a *great* way to get useful text into the docs site.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":198,"context_line":"documented in example configs. The commit message should document the"},{"line_number":199,"context_line":"change for the change log."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Always point out tyops or grammar mistakes you when see them in"},{"line_number":202,"context_line":"review, but also consider that if you were able to recognize the"},{"line_number":203,"context_line":"intent of the statement - documentation with typos may be easier to"},{"line_number":204,"context_line":"iterate and improve on than nothing. ;)"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a3a4534f_32fc088e","line":201,"range":{"start_line":201,"start_character":17,"end_line":201,"end_character":22},"updated":"2022-02-14 09:32:21.000000000","message":"LOL\n\ns/tyops/typographical errors/  :)","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0861f1f978befd32e29a9d717220b4b3a410ef3d","unresolved":true,"context_lines":[{"line_number":199,"context_line":"change for the change log."},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Always point out tyops or grammar mistakes you when see them in"},{"line_number":202,"context_line":"review, but also consider that if you were able to recognize the"},{"line_number":203,"context_line":"intent of the statement - documentation with typos may be easier to"},{"line_number":204,"context_line":"iterate and improve on than nothing. ;)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"If a change does not have adequate documentation it may not be suitable to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fc3c5be_68f36c60","line":203,"range":{"start_line":202,"start_character":31,"end_line":203,"end_character":22},"updated":"2022-02-14 09:38:14.000000000","message":"fair enough, but we need to be mindful of readers who may not recognise the intent - \"good enough for me\" may not be good enough 😊","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b747caa01de8d8e84bb1e3e47ce94a3bd6b16a41","unresolved":true,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"Always point out tyops or grammar mistakes you when see them in"},{"line_number":202,"context_line":"review, but also consider that if you were able to recognize the"},{"line_number":203,"context_line":"intent of the statement - documentation with typos may be easier to"},{"line_number":204,"context_line":"iterate and improve on than nothing. ;)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"If a change does not have adequate documentation it may not be suitable to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"03082fea_929d9e70","line":203,"range":{"start_line":203,"start_character":23,"end_line":203,"end_character":25},"updated":"2022-02-14 09:32:21.000000000","message":"since we\u0027re on the subject of grammar, the use of a hyphen isn\u0027t appropriate here:\n\n  consider that, provided you were able to recognize the\nintent of the statement, documentation with typos may be easier to\niterate and improve on than nothing.","commit_id":"baf5567cb053108319248428834666edbc33cc53"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0861f1f978befd32e29a9d717220b4b3a410ef3d","unresolved":true,"context_lines":[{"line_number":201,"context_line":"Always point out tyops or grammar mistakes you when see them in"},{"line_number":202,"context_line":"review, but also consider that if you were able to recognize the"},{"line_number":203,"context_line":"intent of the statement - documentation with typos may be easier to"},{"line_number":204,"context_line":"iterate and improve on than nothing. ;)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"If a change does not have adequate documentation it may not be suitable to"},{"line_number":207,"context_line":"merge. If a change includes incorrect or misleading documentation or is"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c3443504_afcb2d8b","line":204,"updated":"2022-02-14 09:38:14.000000000","message":"Is it valid, and worth mentioning, that the bar should be set higher for docs vs inline comments?","commit_id":"baf5567cb053108319248428834666edbc33cc53"}]}