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
