Re: [openstack-dev] [style] () vs \ continuations
A little late, but here is the patch to put this into hacking. https://review.openstack.org/#/c/64584/ And here is it running against nova: http://logs.openstack.org/84/64584/1/check/gate-hacking-integration-nova/b31c47e/console.html On Mon, Nov 18, 2013 at 5:23 PM, Vishvananda Ishaya vishvana...@gmail.comwrote: On Nov 14, 2013, at 10:00 AM, Monty Taylor mord...@inaugust.com wrote: On 11/13/2013 08:08 PM, Robert Collins wrote: On 14 November 2013 13:59, Sean Dague s...@dague.net wrote: This is an area where we actually have consensus in our docs (have had for a while), the reviewer was being consistent with them, and it feels like you are reopening that for personal preference. Sorry that it feels that way. My personal code also uses () overwhelmingly - so this isn't a personal agenda issue. I brought it up because the person that wrote the code had chosen to use \, and as far as I knew we didn't have a hard decision either way - and the style guide we have talks preference not requirement, but the review didn't distinguish between whether it's a suggestion or a requirement. I'm seeking clarity so I can review more effectively and so that our code doesn't end up consistent but hard to read. I'd say we've got an expression of clarity here - which means potentially a patch to the hacking guide to clarify the language on what our choice is, as well as the addition of a hacking check to enforce it would be in bounds. +1 to sticking something in hacking. FWIW I would probably do the following to avoid the debate altogether: result = self._path_file_exists(ds_browser, folder_path, file_name) folder_exists, file_exists, file_size_in_kb, disk_extents = result Vish Honestly I find \ at the end of a line ugly as sin, and completely jarring to read. I actually do like the second one better. I don't care enough to change a policy on it, but we do already have a policy, so it seems pretty pedantic, and not useful. Ok, thats interesting. Readability matters, and if most folk find that even this case - which is pretty much the one case where I would argue for \ - is still easier to read with (), then thats cool. Bringing up for debate the style guide every time it disagrees with your personal preference isn't a very effective use of people's time. Especially on settled matters. Totally not what I'm doing. I've been told that much of our style guide was copied lock stock and barrel from some Google Python style guide, so I can't tell what is consensus and what is 'what someone copied down one day'. Particularly when there is no rationale included against the point - its a black box and entirely opaque. -Rob ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On Mon, Nov 18, 2013 at 8:23 PM, Vishvananda Ishaya vishvana...@gmail.comwrote: +1 to sticking something in hacking. FWIW I would probably do the following to avoid the debate altogether: result = self._path_file_exists(ds_browser, folder_path, file_name) folder_exists, file_exists, file_size_in_kb, disk_extents = result Vish I'm working on some test code and came up with what I think is a valid use of a line ending in \. I'm not escaping the newline from a Python syntax point of view. I'm doing in inside of a string literal. Example: mismatches_description = textwrap.dedent(\ expected = test xmlns=http://docs.openstack.org/identity/api/v2.0 success/ /test actual = test xmlns=http://docs.openstack.org/identity/api/v2.0; nope_it_fails/ /test ) This can be written in several different ways, but I think that none of them is as clear as the trailing slash. This is, I think, the clearest alternative example: mismatches_description = textwrap.dedent( expected = test xmlns=http://docs.openstack.org/identity/api/v2.0 success/ /test actual = test xmlns=http://docs.openstack.org/identity/api/v2.0; nope_it_fails/ /test ).lstrip() Would this use be forbidden too? -- David blog: http://www.traceback.org twitter: http://twitter.com/dstanek www: http://dstanek.com ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On Nov 14, 2013, at 10:00 AM, Monty Taylor mord...@inaugust.com wrote: On 11/13/2013 08:08 PM, Robert Collins wrote: On 14 November 2013 13:59, Sean Dague s...@dague.net wrote: This is an area where we actually have consensus in our docs (have had for a while), the reviewer was being consistent with them, and it feels like you are reopening that for personal preference. Sorry that it feels that way. My personal code also uses () overwhelmingly - so this isn't a personal agenda issue. I brought it up because the person that wrote the code had chosen to use \, and as far as I knew we didn't have a hard decision either way - and the style guide we have talks preference not requirement, but the review didn't distinguish between whether it's a suggestion or a requirement. I'm seeking clarity so I can review more effectively and so that our code doesn't end up consistent but hard to read. I'd say we've got an expression of clarity here - which means potentially a patch to the hacking guide to clarify the language on what our choice is, as well as the addition of a hacking check to enforce it would be in bounds. +1 to sticking something in hacking. FWIW I would probably do the following to avoid the debate altogether: result = self._path_file_exists(ds_browser, folder_path, file_name) folder_exists, file_exists, file_size_in_kb, disk_extents = result Vish Honestly I find \ at the end of a line ugly as sin, and completely jarring to read. I actually do like the second one better. I don't care enough to change a policy on it, but we do already have a policy, so it seems pretty pedantic, and not useful. Ok, thats interesting. Readability matters, and if most folk find that even this case - which is pretty much the one case where I would argue for \ - is still easier to read with (), then thats cool. Bringing up for debate the style guide every time it disagrees with your personal preference isn't a very effective use of people's time. Especially on settled matters. Totally not what I'm doing. I've been told that much of our style guide was copied lock stock and barrel from some Google Python style guide, so I can't tell what is consensus and what is 'what someone copied down one day'. Particularly when there is no rationale included against the point - its a black box and entirely opaque. -Rob ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On Wed, Nov 13, 2013 at 6:46 PM, Robert Collins robe...@robertcollins.netwrote: Hi so - in http://docs.openstack.org/developer/hacking/ it has as bullet point 4: Long lines should be wrapped in parentheses in preference to using a backslash for line continuation. I'm seeing in some reviews a request for () over \ even when \ is significantly clearer. I'd like us to avoid meaningless reviewer churn here: can we either: - go with PEP8 which also prefers () but allows \ when it is better - and reviewers need to exercise judgement when asking for one or other - make it a hard requirement that flake8 detects +1 for the non-human approach. My strong recommendation is to go with PEP8 and exercising of judgement. The case that made me raise this is this: folder_exists, file_exists, file_size_in_kb, disk_extents = \ self._path_file_exists(ds_browser, folder_path, file_name) Wrapping that in brackets gets this; folder_exists, file_exists, file_size_in_kb, disk_extents = ( self._path_file_exists(ds_browser, folder_path, file_name)) The root of the problem is that it's a terribly named method with a terrible return value... fix the underlying problem. Which is IMO harder to read - double brackets, but no function call, and no tuple: it's more ambiguous than \. from https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py Cheers, Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On Nov 14, 2013 6:58 AM, Dolph Mathews dolph.math...@gmail.com wrote: On Wed, Nov 13, 2013 at 6:46 PM, Robert Collins robe...@robertcollins.net wrote: Hi so - in http://docs.openstack.org/developer/hacking/ it has as bullet point 4: Long lines should be wrapped in parentheses in preference to using a backslash for line continuation. I'm seeing in some reviews a request for () over \ even when \ is significantly clearer. I'd like us to avoid meaningless reviewer churn here: can we either: - go with PEP8 which also prefers () but allows \ when it is better - and reviewers need to exercise judgement when asking for one or other - make it a hard requirement that flake8 detects +1 for the non-human approach. Humans are a bad match for this type of review work, sounds like we will have to add this into hacking 0.9 My strong recommendation is to go with PEP8 and exercising of judgement. The case that made me raise this is this: folder_exists, file_exists, file_size_in_kb, disk_extents = \ self._path_file_exists(ds_browser, folder_path, file_name) Wrapping that in brackets gets this; folder_exists, file_exists, file_size_in_kb, disk_extents = ( self._path_file_exists(ds_browser, folder_path, file_name)) The root of the problem is that it's a terribly named method with a terrible return value... fix the underlying problem. Which is IMO harder to read - double brackets, but no function call, and no tuple: it's more ambiguous than \. from https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py Cheers, Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On Thu, Nov 14, 2013 at 10:03 AM, Joe Gordon joe.gord...@gmail.com wrote: On Nov 14, 2013 6:58 AM, Dolph Mathews dolph.math...@gmail.com wrote: On Wed, Nov 13, 2013 at 6:46 PM, Robert Collins robe...@robertcollins.net wrote: Hi so - in http://docs.openstack.org/developer/hacking/ it has as bullet point 4: Long lines should be wrapped in parentheses in preference to using a backslash for line continuation. I'm seeing in some reviews a request for () over \ even when \ is significantly clearer. I'd like us to avoid meaningless reviewer churn here: can we either: - go with PEP8 which also prefers () but allows \ when it is better - and reviewers need to exercise judgement when asking for one or other - make it a hard requirement that flake8 detects +1 for the non-human approach. Humans are a bad match for this type of review work, sounds like we will have to add this into hacking 0.9 My strong recommendation is to go with PEP8 and exercising of judgement. The case that made me raise this is this: folder_exists, file_exists, file_size_in_kb, disk_extents = \ self._path_file_exists(ds_browser, folder_path, file_name) Wrapping that in brackets gets this; folder_exists, file_exists, file_size_in_kb, disk_extents = ( self._path_file_exists(ds_browser, folder_path, file_name)) The root of the problem is that it's a terribly named method with a terrible return value... fix the underlying problem. Which is IMO harder to read - double brackets, but no function call, and no tuple: it's more ambiguous than \. from https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py Cheers, Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev personally I don't see the big deal here, I think there can be some judgement etc. BUT it seems to me that this is an awful waste of time. Just automate it one way or the other and let reviewers actually focus on something useful. Frankly I could care less about line separation and am much more concerned about bugs being introduced via patches that reviewers didn't catch. That's ok though, at least the line continuations were correct. Sorry, I shouldn't be a jerk but we seem to have rather pointless debates as of late (spelling/grammar in comments etc etc). IMO we should all do our best on these things but really the focus here should be on the technical components of the code. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On 11/13/2013 08:08 PM, Robert Collins wrote: On 14 November 2013 13:59, Sean Dague s...@dague.net wrote: This is an area where we actually have consensus in our docs (have had for a while), the reviewer was being consistent with them, and it feels like you are reopening that for personal preference. Sorry that it feels that way. My personal code also uses () overwhelmingly - so this isn't a personal agenda issue. I brought it up because the person that wrote the code had chosen to use \, and as far as I knew we didn't have a hard decision either way - and the style guide we have talks preference not requirement, but the review didn't distinguish between whether it's a suggestion or a requirement. I'm seeking clarity so I can review more effectively and so that our code doesn't end up consistent but hard to read. I'd say we've got an expression of clarity here - which means potentially a patch to the hacking guide to clarify the language on what our choice is, as well as the addition of a hacking check to enforce it would be in bounds. Honestly I find \ at the end of a line ugly as sin, and completely jarring to read. I actually do like the second one better. I don't care enough to change a policy on it, but we do already have a policy, so it seems pretty pedantic, and not useful. Ok, thats interesting. Readability matters, and if most folk find that even this case - which is pretty much the one case where I would argue for \ - is still easier to read with (), then thats cool. Bringing up for debate the style guide every time it disagrees with your personal preference isn't a very effective use of people's time. Especially on settled matters. Totally not what I'm doing. I've been told that much of our style guide was copied lock stock and barrel from some Google Python style guide, so I can't tell what is consensus and what is 'what someone copied down one day'. Particularly when there is no rationale included against the point - its a black box and entirely opaque. -Rob ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On 11/13/2013 07:46 PM, Robert Collins wrote: Hi so - in http://docs.openstack.org/developer/hacking/ it has as bullet point 4: Long lines should be wrapped in parentheses in preference to using a backslash for line continuation. I'm seeing in some reviews a request for () over \ even when \ is significantly clearer. I'd like us to avoid meaningless reviewer churn here: can we either: - go with PEP8 which also prefers () but allows \ when it is better - and reviewers need to exercise judgement when asking for one or other - make it a hard requirement that flake8 detects My strong recommendation is to go with PEP8 and exercising of judgement. The case that made me raise this is this: folder_exists, file_exists, file_size_in_kb, disk_extents = \ self._path_file_exists(ds_browser, folder_path, file_name) Wrapping that in brackets gets this; folder_exists, file_exists, file_size_in_kb, disk_extents = ( self._path_file_exists(ds_browser, folder_path, file_name)) Which is IMO harder to read - double brackets, but no function call, and no tuple: it's more ambiguous than \. from https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py This is an area where we actually have consensus in our docs (have had for a while), the reviewer was being consistent with them, and it feels like you are reopening that for personal preference. Honestly I find \ at the end of a line ugly as sin, and completely jarring to read. I actually do like the second one better. I don't care enough to change a policy on it, but we do already have a policy, so it seems pretty pedantic, and not useful. Bringing up for debate the style guide every time it disagrees with your personal preference isn't a very effective use of people's time. Especially on settled matters. -Sean -- Sean Dague http://dague.net signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
I'd like us to avoid meaningless reviewer churn here: I'd like us to avoid trivial style guideline churn :) The case that made me raise this is this: folder_exists, file_exists, file_size_in_kb, disk_extents = \ self._path_file_exists(ds_browser, folder_path, file_name) Wrapping that in brackets gets this; folder_exists, file_exists, file_size_in_kb, disk_extents = ( self._path_file_exists(ds_browser, folder_path, file_name)) Which is IMO harder to read - double brackets, but no function call, and no tuple: it's more ambiguous than \. I prefer consistency for readability over most everything. In Nova, we have a few cases of backslash continuations, which I think are mostly in old db_api code, but I think it's overwhelmingly paren-based continuations. I'd much rather keep things the way they are except for situations where there is a real problem. I think that when modifying existing backslash-using code, nobody argues, and I think that if an author were to make a reasonable readability argument in a specific case, that reviewers would allow the backslash method. --Dan ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On 14 November 2013 13:59, Sean Dague s...@dague.net wrote: This is an area where we actually have consensus in our docs (have had for a while), the reviewer was being consistent with them, and it feels like you are reopening that for personal preference. Sorry that it feels that way. My personal code also uses () overwhelmingly - so this isn't a personal agenda issue. I brought it up because the person that wrote the code had chosen to use \, and as far as I knew we didn't have a hard decision either way - and the style guide we have talks preference not requirement, but the review didn't distinguish between whether it's a suggestion or a requirement. I'm seeking clarity so I can review more effectively and so that our code doesn't end up consistent but hard to read. Honestly I find \ at the end of a line ugly as sin, and completely jarring to read. I actually do like the second one better. I don't care enough to change a policy on it, but we do already have a policy, so it seems pretty pedantic, and not useful. Ok, thats interesting. Readability matters, and if most folk find that even this case - which is pretty much the one case where I would argue for \ - is still easier to read with (), then thats cool. Bringing up for debate the style guide every time it disagrees with your personal preference isn't a very effective use of people's time. Especially on settled matters. Totally not what I'm doing. I've been told that much of our style guide was copied lock stock and barrel from some Google Python style guide, so I can't tell what is consensus and what is 'what someone copied down one day'. Particularly when there is no rationale included against the point - its a black box and entirely opaque. -Rob -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [style] () vs \ continuations
On 14 November 2013 15:28, Chris Behrens cbehr...@codestud.com wrote: For this example, I'd look at using parens on the left side to see if that helps. I also, like Sean, really dislike the look of \. I dislike \ most of the time too :). (foo, bar, baz = File stdin, line 1 (foo, bar, baz = ^ SyntaxError: invalid syntax -- Robert Collins rbtcoll...@hp.com Distinguished Technologist HP Converged Cloud ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
