Open Stack

On Thu, Nov 14, 2013 at 10:03 AM, Joe Gordon <joe.gordon0 at gmail.com> wrote:
>
> On Nov 14, 2013 6:58 AM, "Dolph Mathews" <dolph.mathews at gmail.com> wrote:
>>
>>
>> On Wed, Nov 13, 2013 at 6:46 PM, Robert Collins
>> <robertc at 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 <rbtcollins at hp.com>
>>> Distinguished Technologist
>>> HP Converged Cloud
>>>
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>>
>>
>> --
>>
>> -Dolph
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at 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.