<p dir="ltr"><br>
On Nov 14, 2013 6:58 AM, "Dolph Mathews" <<a href="mailto:dolph.mathews@gmail.com">dolph.mathews@gmail.com</a>> wrote:<br>
><br>
><br>
> On Wed, Nov 13, 2013 at 6:46 PM, Robert Collins <<a href="mailto:robertc@robertcollins.net">robertc@robertcollins.net</a>> wrote:<br>
>><br>
>> Hi so - in <a href="http://docs.openstack.org/developer/hacking/">http://docs.openstack.org/developer/hacking/</a><br>
>><br>
>> it has as bullet point 4:<br>
>> Long lines should be wrapped in parentheses in preference to using a<br>
>> backslash for line continuation.<br>
>><br>
>> I'm seeing in some reviews a request for () over \ even when \ is<br>
>> significantly clearer.<br>
>><br>
>> I'd like us to avoid meaningless reviewer churn here: can we either:<br>
>>  - go with PEP8 which also prefers () but allows \ when it is better<br>
>>    - and reviewers need to exercise judgement when asking for one or other<br>
>>  - make it a hard requirement that flake8 detects<br>
><br>
><br>
> +1 for the non-human approach.</p>
<p dir="ltr">Humans are a bad match for this type of review work, sounds like we will have to add this into hacking 0.9</p>
<p dir="ltr">>  <br>
>><br>
>><br>
>> My strong recommendation is to go with PEP8 and exercising of judgement.<br>
>><br>
>> The case that made me raise this is this:<br>
>>     folder_exists, file_exists, file_size_in_kb, disk_extents = \<br>
>>         self._path_file_exists(ds_browser, folder_path, file_name)<br>
>><br>
>> Wrapping that in brackets gets this;<br>
>>     folder_exists, file_exists, file_size_in_kb, disk_extents = (<br>
>>         self._path_file_exists(ds_browser, folder_path, file_name))<br>
><br>
><br>
> The root of the problem is that it's a terribly named method with a terrible return value... fix the underlying problem.<br>
>  <br>
>><br>
>><br>
>> Which is IMO harder to read - double brackets, but no function call,<br>
>> and no tuple: it's more ambiguous than \.<br>
>><br>
>> from <a href="https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py">https://review.openstack.org/#/c/48544/15/nova/virt/vmwareapi/vmops.py</a><br>
>><br>
>> Cheers,<br>
>> Rob<br>
>> --<br>
>> Robert Collins <<a href="mailto:rbtcollins@hp.com">rbtcollins@hp.com</a>><br>
>> Distinguished Technologist<br>
>> HP Converged Cloud<br>
>><br>
>> _______________________________________________<br>
>> OpenStack-dev mailing list<br>
>> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
><br>
><br>
><br>
> -- <br>
><br>
> -Dolph<br>
><br>
> _______________________________________________<br>
> OpenStack-dev mailing list<br>
> <a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
><br>
</p>